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

test(NODE-4534): unified SDAM test runner implementation #3373

Merged
merged 19 commits into from
Aug 24, 2022

Conversation

dariakp
Copy link
Contributor

@dariakp dariakp commented Aug 18, 2022

Description

NODE-4534 / NODE-4325

What is changing?

Converting SDAM tests to unified format

  • Implementing the createEntities operation from unified schema 1.9
  • Implementing unified schema 1.10 support for thread entities (runOnThread, waitForThread), TopologyDescription entities (recordTopologyDescription, waitForPrimaryChange, assertTopologyType), testRunner event assertion operations (waitForEvent, assertEventCount), expected SDAM events, and the wait operation
  • Sync unified SDAM tests and remove integration folder
  • Updated the empty CMAP event matcher to actually assert the event type
  • Sync invalid schema tests with latest (we don't run them)

Most interesting changes:

  • waitForEvent implementation
  • UnifiedThread implementation
  • socket leak check and cleanup afterEach hook
Is there new documentation needed for these changes?

No

What is the motivation for this change?

Spec compliance and more reliable tests

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>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@dariakp dariakp added the wip label Aug 18, 2022
@dariakp dariakp changed the title test(NODE-4534): sync latest invalid unified tests test(NODE-4534): unified SDAM test runner implementation Aug 22, 2022
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.

Looks great! just some small comments but really appreciate how you've done sleepOrError style assertions

test/tools/unified-spec-runner/entities.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/entities.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/entities.ts Show resolved Hide resolved
test/tools/unified-spec-runner/match.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/operations.ts Show resolved Hide resolved
test/tools/unified-spec-runner/operations.ts Outdated Show resolved Hide resolved
@dariakp dariakp force-pushed the NODE-4325/sdam-unified-format branch from 9b7ea19 to 4add27a Compare August 22, 2022 22:14
@dariakp dariakp force-pushed the NODE-4325/sdam-unified-format branch from 2d1432c to 7702e63 Compare August 22, 2022 22:50
@dariakp dariakp added Primary Review In Review with primary reviewer, not yet ready for team's eyes and removed wip labels Aug 23, 2022
@dariakp dariakp marked this pull request as ready for review August 23, 2022 15:50
@nbbeeken nbbeeken self-requested a review August 23, 2022 16:26
test/tools/unified-spec-runner/entities.ts Show resolved Hide resolved
test/tools/unified-spec-runner/entities.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/match.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/match.ts Show resolved Hide resolved
test/tools/unified-spec-runner/operations.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/schema.ts Outdated Show resolved Hide resolved
test/tools/unified-spec-runner/unified-utils.ts Outdated Show resolved Hide resolved
@dariakp dariakp requested a review from nbbeeken August 23, 2022 21:15
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

@nbbeeken nbbeeken added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Aug 24, 2022
@dariakp
Copy link
Contributor Author

dariakp commented Aug 24, 2022

The test failure is the op count latency test that Bailey is taking care of

@nbbeeken nbbeeken merged commit 6a86553 into main Aug 24, 2022
@nbbeeken nbbeeken deleted the NODE-4325/sdam-unified-format branch August 24, 2022 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
2 participants