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

Make pytest fail when tasks are running after a test #2927

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

benclifford
Copy link
Collaborator

@benclifford benclifford commented Oct 26, 2023

When pressing ctrl-c in pytest, prior to this PR, it would hang as wait_for_task_completion would be called in the unwinding of the fixture stack. However, because ctrl-c was pressed, tasks wouldn't be expected to all complete.

This PR changes that to assert that all tasks are completed, rather than waiting for all tasks to complete.

A non-finished task now gives a test error - which is arguably better anyway because it more aggressively flushes out tests that do not perform a complete shutdown.

This means that pressing ctrl-C in a pytest leads to an assertion error; when previously it led to a hang.

One recently introduced test is fixed to conform to this new requirement.

This is part of safe-shutdown work.

Type of change

  • New feature (non-breaking change that adds functionality)
  • Code maintentance/cleanup

Copy link
Collaborator

@khk-globus khk-globus left a comment

Choose a reason for hiding this comment

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

Och; I'm very much in favor of this change. Well-explained rationale as well. Thank you for this.

When pressing ctrl-c in pytest, prior to this PR, it would hang as
wait_for_task_completion would be called in the unwinding of the
fixture stack. However, because ctrl-c was pressed, tasks wouldn't be
expected to all complete.

This PR changes that to assert that all tasks are completed, rather
than *waiting* for all tasks to complete.

A non-finished task now gives a test error - which is arguably better
anyway because it more aggressively flushes out tests that do not perform
a complete shutdown.

This means that pressing ctrl-C in a pytest leads to an assertion error;
when previously it led to a hang.

One recently introduced test is fixed to

This is part of safe-shutdown work.
@khk-globus khk-globus force-pushed the benc-tests-no-running-tasks branch from 81be8e5 to 488aa37 Compare November 1, 2023 15:19
@benclifford benclifford merged commit 0cbf96d into master Nov 1, 2023
4 checks passed
@benclifford benclifford deleted the benc-tests-no-running-tasks branch November 1, 2023 18:40
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.

2 participants