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

remove pytest-asyncio #6063

Merged
merged 12 commits into from
Apr 8, 2022
Merged

remove pytest-asyncio #6063

merged 12 commits into from
Apr 8, 2022

Conversation

graingert
Copy link
Member

@graingert graingert commented Apr 4, 2022

gen_test doesn't transport marks correctly and so must be the first
decorator applied to a test function

also gen_test is stricter than pytest-asyncio it checks for leaky comms and
threads, so we need to make sure resources are closed properly

@graingert graingert force-pushed the remove-pytest-asyncio branch 3 times, most recently from 6991040 to 8042a7a Compare April 4, 2022 17:17
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2022

Unit Test Results

       18 files  ±0         18 suites  ±0   8h 51m 53s ⏱️ - 12m 42s
  2 724 tests ±0    2 640 ✔️ ±0       81 💤  - 1  3 +1 
24 364 runs  ±0  23 143 ✔️  - 2  1 217 💤 ±0  4 +2 

For more details on these failures, see this check.

Results for commit 70ca320. ± Comparison against base commit 6e30766.

♻️ This comment has been updated with latest results.

@graingert graingert force-pushed the remove-pytest-asyncio branch 2 times, most recently from 61554fe to 15982cc Compare April 5, 2022 11:10
Comment on lines -1060 to -1050
async with await LocalCluster(
typo_kwarg="foo",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the asynchronous=True kwarg was missing so this spawned a new thread

@graingert graingert marked this pull request as ready for review April 5, 2022 16:07
@sjperkins sjperkins self-requested a review April 5, 2022 16:20
@graingert graingert requested a review from fjetter April 6, 2022 08:26
@graingert
Copy link
Member Author

the macos failures seem unrelated

distributed/deploy/tests/test_local.py Outdated Show resolved Hide resolved
distributed/deploy/tests/test_local.py Show resolved Hide resolved
distributed/comm/tests/test_ucx.py Show resolved Hide resolved
distributed/tests/test_batched.py Show resolved Hide resolved
distributed/tests/test_core.py Show resolved Hide resolved
assert s.foo == 1
assert "12345/preload" in log.getvalue()
with socket.create_server(("127.0.0.1", 0)) as server_sock:
await asyncio.gather(to_thread(preload), test())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a more robust expression of the previous test, achieved by:

  1. Avoiding creating the preload application in a separate process
  2. Allowing a dynamically allocated port

distributed/tests/test_worker.py Show resolved Hide resolved
@fjetter
Copy link
Member

fjetter commented Apr 6, 2022

OSX 3.9 not ci1 failure #6076

@graingert graingert requested a review from fjetter April 6, 2022 10:37
Copy link
Member

@fjetter fjetter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once builds pass

@graingert
Copy link
Member Author

graingert commented Apr 7, 2022

test_client.py::test_reconnect timed out in py3.10 ci1 see also #6000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants