Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flaky visual test: Loading #578

Open
pathunstrom opened this issue Feb 20, 2021 · 6 comments
Open

Flaky visual test: Loading #578

pathunstrom opened this issue Feb 20, 2021 · 6 comments
Labels
tests Related to tests. When applied to PRs is a note that tests are required before merge.

Comments

@pathunstrom
Copy link
Collaborator

ppb commit: b2eee07
OS Windows 10
Python 3.8.6

Output:

==========
loading.py
==========

Tests loading scenes.

Should take some time to progress. This may need to be run several times to suss
out threading bugs.

UserWarning: Using SDL2 binaries from pysdl2-dll 2.0.10
bullet.png 1.527485162932843
player.png 4.096429452946413
target.png 5.921888454316235
AssetLoaded(asset=<Image name='target.png' loaded at 0x149caf40220>, total_loaded=1, total_queued=0)
Traceback (most recent call last):
  File "C:\Users\pathu\PycharmProjects\pursuedpybear\viztests\loading.py", line 60, in <module>
    ppb.run(starting_scene=LoadingScene)
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\__init__.py", line 125, in run
    eng.run()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 311, in run
    self.main_loop()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 340, in main_loop
    self.loop_once()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 356, in loop_once
    self.publish()
  File "c:\users\pathu\pycharmprojects\pursuedpybear\ppb\engine.py", line 382, in publish
    method(event, self.signal)
  File "C:\Users\pathu\PycharmProjects\pursuedpybear\viztests\loading.py", line 37, in on_update
    assert self.bullet.is_loaded()
AssertionError
Traceback (most recent call last):
  File "C:\Users\pathu\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\pathu\AppData\Local\Programs\Python\Python38\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Users\pathu\PycharmProjects\pursuedpybear\viztests\__main__.py", line 24, in <module>
    subprocess.run([sys.executable, str(script)], check=True)
  File "C:\Users\pathu\AppData\Local\Programs\Python\Python38\lib\subprocess.py", line 512, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['C:\\Users\\pathu\\PycharmProjects\\pursuedpybear\\venv\\Scripts\\python.exe', 'C:\\Users\\pathu\\PycharmProjects\\pursuedpybe
ar\\viztests\\loading.py']' returned non-zero exit status 1.

Some kind of race condition I think? It's the assert failing.

@pathunstrom pathunstrom added the tests Related to tests. When applied to PRs is a note that tests are required before merge. label Feb 21, 2021
@AstraLuma
Copy link
Member

This may need to be run several times to suss out threading bugs.

Thanks past me for noting this and trying to write a test to induce this.

Present us now have to debug multi-threaded code.

@mark-boer
Copy link
Contributor

mark-boer commented Feb 21, 2021

Interestingly, the problem seems to be that the "target.png" is finished loading, before the three delayed images have started loading. (So surprisingly it seems that the problem does not arise due to the sleep) Therefore the loading screen, thinks it has loaded all 1 of 1 images and thus continues to the quitter screen.

You could fix it by starting the threadpool later, after all AssetLoading jobs have been submitted. Or maybe better not using the total_queued in the event, but instead look at the global ThreadPool._finished from the LoadingScene.on_asset_loaded.

Also the DelayedThreadExecutor._finish is run from the worker threads, which also touch the _finished and _started variables. It's read only, so it shouldn't be too bad, but it could lead to race conditions.

Edit: fix typo's

@AstraLuma
Copy link
Member

New assets can be loaded at any time. And I'd really rather that the loading scene not reach into the internals of the asset loading system.

@mark-boer
Copy link
Contributor

You could maybe register assets, that you would want to wait on in the loading screen?

Also calling the DelayedThreadExecutor._finish on the main thread might work. Or checking finished jobs in the main loop?

Or, maybe not use the data in the AssetLoaded event, but instead in on_asset_loaded, check if all the assets in the next scene are loaded.

@AstraLuma
Copy link
Member

You could maybe register assets, that you would want to wait on in the loading screen?

Gotta make sure you both reference the asset and then pre-declare it for the loading screen? And then you forget to update one of the two places.

Also calling the DelayedThreadExecutor._finish on the main thread might work. Or checking finished jobs in the main loop?

I mean, it's never actually finished, just idle. Again, new, previously unknown assets can be referenced at any time.

Or, maybe not use the data in the AssetLoaded event, but instead in on_asset_loaded, check if all the assets in the next scene are loaded.

How do you make that list?

@mark-boer
Copy link
Contributor

I'm just spitballing here, I don't actually know what would make most sense in this case.

Gotta make sure you both reference the asset and then pre-declare it for the loading screen?

It would be difficult, because asset classes are not instantiated until the scenes are instantiated.

How do you make that list?

You could use meta classes, similar to how SQLAlchemy ORM is used. But I don't know if that would work with the current syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Related to tests. When applied to PRs is a note that tests are required before merge.
Projects
None yet
Development

No branches or pull requests

3 participants