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.

Sign up for GitHub

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

Jump to bottom

test(NODE-6620): client.close() interrupts file reads #4355

Merged
merged 30 commits into from
Jan 8, 2025
Merged

test(NODE-6620): client.close() interrupts file reads #4355

merged 30 commits into from
Jan 8, 2025
+526 −15

Conversation

aditi-khare-mongoDB
Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB commented Dec 16, 2024 •
edited
Loading

Description

Add test matrix for each node.js and server resource creation/clean-up scenario that is handled by client.close.

What is changing?

  • Testing skeleton is completd, but skipped since actual client.close() implementation is not completed yet.
  • Added process launching script for asserting Node.js resource clean-up
  • Implemented File Read Tests
    • other resource (socket, timer, and server-side) tests will be in separate PRs so reviewing is smoother.
Is there new documentation needed for these changes?

No.

What is the motivation for this change?

To ensure alignment on testing goals for client.close().

Release Highlight

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@aditi-khare-mongoDB aditi-khare-mongoDB changed the title test(NODE-6615): Add integration tests that assert client.close() cleans up all resources test(NODE-6619): Add integration test matrix for improves client.close() Dec 17, 2024
@evergreen-ci-prod evergreen-ci-prod bot mentioned this pull request Dec 17, 2024
Closed
5 tasks
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title test(NODE-6619): Add integration test matrix for improves client.close() test(NODE-6615): Add integration test for improved client.close() Dec 18, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title test(NODE-6615): Add integration test for improved client.close() test(NODE-6615): Add integration tests for improved client.close() Dec 18, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title test(NODE-6615): Add integration tests for improved client.close() test(NODE-6620): Implement integration tests for improved client.close() Dec 20, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title test(NODE-6620): Implement integration tests for improved client.close() test(NODE-6620): Implement integration tests for improved client.close() - Setup and File Reads Dec 30, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB marked this pull request as ready for review December 30, 2024 17:01
@baileympearson baileympearson self-assigned this Dec 30, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB deleted the NODE-6615/integration-client-close branch January 2, 2025 20:21
@aditi-khare-mongoDB aditi-khare-mongoDB restored the NODE-6615/integration-client-close branch January 2, 2025 20:22
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title test(NODE-6620): Implement integration tests for improved client.close() - Setup and File Reads test(NODE-6620): implement integration tests for improved client.close() - setup and file reads Jan 2, 2025
nbbeeken
nbbeeken requested changes Jan 3, 2025
View reviewed changes
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

okay, one last tiny fixup and two optional things from me

test/integration/node-specific/resource_tracking_script_builder.ts Outdated Show resolved Hide resolved
test/integration/node-specific/resource_tracking_script_builder.ts Show resolved Hide resolved
test/integration/node-specific/resource_tracking_script_builder.ts Show resolved Hide resolved
nbbeeken
nbbeeken previously approved these changes Jan 3, 2025
View reviewed changes
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

LGTM

baileympearson
baileympearson previously approved these changes Jan 3, 2025
View reviewed changes
@aditi-khare-mongoDB @addaleax
Update test/integration/node-specific/resource_tracking_script_builde…
aefa1cc 
…r.ts

Co-authored-by: Anna Henningsen <anna@addaleax.net>
@baileympearson baileympearson changed the title test(NODE-6620): implement integration tests for improved client.close() - setup and file reads test(NODE-6620): client.close() interrupts file reads Jan 8, 2025
@baileympearson baileympearson merged commit 3216d33 into main Jan 8, 2025
29 of 31 checks passed
@baileympearson baileympearson deleted the NODE-6615/integration-client-close branch January 8, 2025 16:18
@W-A-James W-A-James mentioned this pull request Jan 14, 2025
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@baileympearson baileympearson baileympearson approved these changes

@nbbeeken nbbeeken nbbeeken approved these changes

@addaleax addaleax addaleax approved these changes

Assignees

@baileympearson baileympearson

Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aditi-khare-mongoDB @addaleax @nbbeeken @baileympearson