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

Check before test ioloop close #1763

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Check before test ioloop close #1763

merged 1 commit into from
Jan 16, 2025

Conversation

khk-globus
Copy link
Contributor

@khk-globus khk-globus commented Jan 15, 2025

  • remove an unwarranted test reconnect() (makes flow much simpler to verify)
  • don't forget to remove reference to connection so it can be GC'd
  • remove redundant shutdown requests, given fixtures
  • fix test to let ioloop know that connection is closing, allows for a cleaner shutdown in the ioloop internals.

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 15, 2025
@khk-globus khk-globus force-pushed the close_test_pika_ioloops branch 3 times, most recently from 61415e5 to 8218488 Compare January 15, 2025 21:29
- remove an unwarranted test `reconnect()` (makes flow much simpler to verify)
- don't forget to remove reference to connection so it can be GC'd
- remove redundant shutdown requests, given fixtures
- fix test to let ioloop know that connection is closing, allows for a cleaner
  shutdown in the ioloop internals.
@khk-globus khk-globus force-pushed the close_test_pika_ioloops branch from 8218488 to 6e15241 Compare January 16, 2025 18:10
@khk-globus khk-globus changed the title Close the test ioloops Check before test ioloop close Jan 16, 2025
Copy link
Contributor

@LeiGlobus LeiGlobus left a comment

Choose a reason for hiding this comment

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

Think it looks good, and makes things a lot cleaner overall.

@khk-globus khk-globus merged commit 4e7d012 into main Jan 16, 2025
21 checks passed
@khk-globus khk-globus deleted the close_test_pika_ioloops branch January 16, 2025 18:47
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