Skip to content

chore(NODE-4348): enhance skip reason filtering in unified runner #3303

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
merged 7 commits into from
Jun 29, 2022

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Jun 27, 2022

Description

What is changing?

This PR enhances the skip test functionality of the unified runner. Rather than accepting an array of tests to skip, the runner now takes a function that is evaluated to determine whether a test should be skipped.

This PR also updates the existing usages of runUnifedSuite that skip tests to use the new function approach.

Finally, some tests relating to auto connect and change stream resumability were unskipped.

Is there new documentation needed for these changes?

Negative.

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

@baileympearson baileympearson requested a review from nbbeeken June 27, 2022 19:38
@baileympearson baileympearson changed the title chore: enhance skip reason filtering in unified runner chore(NODE-4348): enhance skip reason filtering in unified runner Jun 28, 2022
@baileympearson baileympearson marked this pull request as ready for review June 28, 2022 18:45
@baileympearson baileympearson requested a review from nbbeeken June 28, 2022 19:04
nbbeeken
nbbeeken previously approved these changes Jun 28, 2022
@nbbeeken nbbeeken added the Team Review Needs review from team label Jun 28, 2022
Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

Just the one comment which doesn't change the functionality just a general design nit for return values. Feel free to take it as a LGTM if anyone feels strongly against my input.

@durran
Copy link
Member

durran commented Jun 28, 2022

Also 1 lint issue.

- fix lint error
- update type to return false instead of null when tests should be run
Copy link
Contributor

@dariakp dariakp left a comment

Choose a reason for hiding this comment

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

Nicely done

@baileympearson baileympearson merged commit 0ea43c6 into main Jun 29, 2022
@baileympearson baileympearson deleted the NODE-4348 branch June 29, 2022 16:11
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
Development

Successfully merging this pull request may close these issues.

4 participants