Skip to content

GODRIVER-2068 Use monitor.TestPoolMonitor to replace most connection pool monitors in tests. #890

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

Merged

Conversation

matthewdale
Copy link
Collaborator

@matthewdale matthewdale commented Mar 25, 2022

GODRIVER-2068

There are various test types that are used as an *event.PoolMonitor and all work slightly differently with different levels of isolation between tests. The existing testPoolMonitor in the mongo/integration package tests can satisfy the use cases of all pool monitors used in tests and is intended to be instantiated per-test to prevent interaction between tests. Move the existing testPoolMonitor into a new test utility package internal/testutil/monitor and use it in most tests that require a connection pool monitor.

Making the new test pool monitor available to all tests also allows us to improve the flaky TestServerConnectionTimeout test by replacing the test-specific pool monitor with a monitor.TestPoolMonitor.

Changes:

  • Remove the various test types that are used as an *event.PoolMonitor and replace them with the new monitor.TestPoolMonitor type.
  • Remove unused code from the internal/testutil package.

@matthewdale matthewdale force-pushed the godriver2068-test-pool-monitor branch 2 times, most recently from 900c091 to 0e0418c Compare March 30, 2022 18:11
@matthewdale matthewdale marked this pull request as ready for review March 30, 2022 18:12
@matthewdale matthewdale force-pushed the godriver2068-test-pool-monitor branch from 0e0418c to ba528f4 Compare March 30, 2022 18:28
Copy link
Member

@prestonvasquez prestonvasquez left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Awesome! This is great cleanup and putting TestPoolMonitor in its own, new package seems best. Great work 🧑‍🔧

@matthewdale matthewdale merged commit 3d60e91 into mongodb:master Apr 1, 2022
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