Skip to content

Fix the Github workflow tests - #72

Merged
antoine-tran merged 3 commits into
mainfrom
up-tests-version
Jan 15, 2025
Merged

Fix the Github workflow tests#72
antoine-tran merged 3 commits into
mainfrom
up-tests-version

Conversation

@avidale

@avidale avidale commented Jan 15, 2025

Copy link
Copy Markdown
Contributor

Why ?

The tests seem to be falling on main, see e.g. #71.
For the global mining test, the problem seems to be with the interaction of the test and the dependencies (maybe pytest-asyncio?), because the code itself has not changed since long ago.

How ?

The falling test stopes/pipelines/tests/test_global_mining.py was complaining about not having access to an event loop in some of its parametrized runs; apparently, the event gets closed after the first test run.

Setting its asyncio test scope to session seems to fix the problem. I also remove a deprecation warning by setting explicitly asyncio_default_fixture_loop_scope = "session" in the project configuration.

Test plan

Use the automatic Github workflow to run the tests.

Before the changes (the first commit, a dummy commit): there is a warning on the top of the tests log PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset, and the global mining test is failing with RuntimeError: There is no current event loop in thread 'MainThread'..

After the changes: the warning is gone and the tests are passing.

Signed-off-by: David Dale <daviddale@meta.com>
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 15, 2025
Signed-off-by: David Dale <daviddale@meta.com>
Signed-off-by: David Dale <daviddale@meta.com>
@avidale avidale changed the title [WIP] fix the Github workflow tests Fix the Github workflow tests Jan 15, 2025
@avidale
avidale marked this pull request as ready for review January 15, 2025 10:40
@antoine-tran
antoine-tran self-requested a review January 15, 2025 10:46
@antoine-tran
antoine-tran merged commit aeea36f into main Jan 15, 2025
@antoine-tran
antoine-tran deleted the up-tests-version branch January 15, 2025 10:46
@antoine-tran antoine-tran mentioned this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants