-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
test(NODE-6631): Implement integration tests for improved client.close() - sockets + timers #4361
base: main
Are you sure you want to change the base?
Conversation
…cript vars/functions
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
617e9af
to
13a7f27
Compare
54579bd
to
78f787f
Compare
…r.ts Co-authored-by: Anna Henningsen <anna@addaleax.net>
028bf63
to
2a43e4d
Compare
!originalReportAddresses.includes(resource.address) && | ||
resource.is_referenced && // if a resource is unreferenced, it's not keeping the event loop open | ||
(!serverType.includes(resource.type) || resource.is_active) | ||
!originalReportAddresses.includes(resource.address) && resource.is_referenced // if a resource is unreferenced, it's not keeping the event loop open |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up from NODE-6620: We do want to clean up inactive sockets
|
||
const servers = client.topology?.s.servers; | ||
|
||
// note: minPoolSizeCheckFrequencyMS = 100 ms by client, so this test has a chance of being flaky |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the note on the comment. This test has a higher chance of being flaky since the client can't set minPoolSizeCheckFrequencyMS
. I can this to set the variable to a higher value using sinon if preferred by reviewers.
9793c44
to
f003454
Compare
Description
Add socket integration tests for client.close().
What is changing?
libuv
tracking for each relevant driver instance of socket creation.process.getActiveResourceInfo()
MonitorInterval
's timer exitsTo see more information regarding testing plan outline see the design document, specifically the dropdowns under Sockets and Timers sections, respectively.
Is there new documentation needed for these changes?
No
What is the motivation for this change?
Improved client.close()
Release Highlight
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript