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

Convert test class to threading #1764

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

khk-globus
Copy link
Contributor

There's no need to create an external process to run the AMQP test class; in fact, doing so has hidden a couple of subtle pika interaction bugs. Consequently, though the motivating change in this commit is:

- multiprocessing
+ threading/queue.SimpleQueue

there are also some fixes in test/.../result_queue_subscriber around the proper shutdown procedure of the pika.SelectConnection: don't stop the loop prematurely, and close all resources.

Type of change

  • Code maintenance/cleanup

@khk-globus khk-globus added the no-news-is-good-news This change does not require a news file label Jan 16, 2025
There's no need to create an external process to run the AMQP test class; in
fact, doing so has hidden a couple of subtle pika interaction bugs.
Consequently, though the motivating change in this commit is:

    - multiprocessing
    + threading/queue.SimpleQueue

there are also some fixes in `test/.../result_queue_subscriber` around the
proper shutdown procedure of the `pika.SelectConnection`: don't stop the loop
prematurely, and close all resources.
@khk-globus khk-globus force-pushed the simplify_test_support_to_thread branch from 3852c03 to a8245b0 Compare January 16, 2025 17:34
@khk-globus khk-globus merged commit 6d384e7 into main Jan 16, 2025
20 of 21 checks passed
@khk-globus khk-globus deleted the simplify_test_support_to_thread branch January 16, 2025 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-news-is-good-news This change does not require a news file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants