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

Added support for passing a custom --testMatch option to Jest #338

Merged
merged 4 commits into from
Feb 20, 2023

Conversation

Xiltyn
Copy link
Collaborator

@Xiltyn Xiltyn commented Feb 7, 2023

Summary

Resolves: #314

  • Added new option to yarn reassure measure and subsequently - yarn reassure commands: --file | -f
  • Added validation for the option by checking whether provided file exists
  • Fallback to default if file does not exist

Important
Using TEST_RUNNER_ARGS will override this feature as it works by adjusting the defaultArgs string

Test plan

  1. Clone repo
  2. run yarn
  3. run yarn reassure -f "examples/native/src/OtherTest.perf-test.tsx"

Result: Only one test is ran

Demo

reassure-cli-single-file.mov

@changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2023

⚠️ No Changeset found

Latest commit: 6c41d08

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2023

Performance Comparison Report

Significant Changes To Render Duration

Name Render Duration Render Count
Other Component 10 legacy scenario 150.0 ms → 145.2 ms (-4.8 ms, -3.2%) 4 → 4
Show details
Name Render Duration Render Count
Other Component 10 legacy scenario Baseline
Mean: 150.0 ms
Stdev: 4.4 ms (2.9%)
Runs: 157 157 152 150 150 149 148 148 146 143

Current
Mean: 145.2 ms
Stdev: 4.1 ms (2.8%)
Runs: 151 149 148 147 147 145 143 143 142 137
Baseline
Mean: 4
Stdev: 0 (0.0%)
Runs: 4 4 4 4 4 4 4 4 4 4

Current
Mean: 4
Stdev: 0 (0.0%)
Runs: 4 4 4 4 4 4 4 4 4 4

Meaningless Changes To Render Duration

Show entries
Name Render Duration Render Count
Async Component 240.7 ms → 241.6 ms (+0.9 ms, ±0.0%) 7 → 7
Other Component 10 151.9 ms → 152.4 ms (+0.5 ms, ±0.0%) 4 → 4
Other Component 20 151.3 ms → 149.1 ms (-2.3 ms, -1.5%) 4 → 4
Show details
Name Render Duration Render Count
Async Component Baseline
Mean: 240.7 ms
Stdev: 6.1 ms (2.5%)
Runs: 249 248 244 244 242 242 238 236 233 231

Current
Mean: 241.6 ms
Stdev: 9.8 ms (4.1%)
Runs: 261 254 245 242 242 238 237 237 231 229
Baseline
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7

Current
Mean: 7
Stdev: 0 (0.0%)
Runs: 7 7 7 7 7 7 7 7 7 7
Other Component 10 Baseline
Mean: 151.9 ms
Stdev: 3.9 ms (2.6%)
Runs: 158 157 156 153 151 150 150 149 148 147

Current
Mean: 152.4 ms
Stdev: 3.1 ms (2.1%)
Runs: 157 157 154 153 153 152 151 151 148 148
Baseline
Mean: 4
Stdev: 0 (0.0%)
Runs: 4 4 4 4 4 4 4 4 4 4

Current
Mean: 4
Stdev: 0 (0.0%)
Runs: 4 4 4 4 4 4 4 4 4 4
Other Component 20 Baseline
Mean: 151.3 ms
Stdev: 5.2 ms (3.5%)
Runs: 162 158 155 155 155 154 153 153 153 153 153 151 150 150 148 148 148 146 141 140

Current
Mean: 149.1 ms
Stdev: 7.3 ms (4.9%)
Runs: 161 157 155 154 154 154 154 153 153 152 150 149 147 146 145 145 143 142 135 132
Baseline
Mean: 4
Stdev: 0 (0.0%)
Runs: 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4

Current
Mean: 4
Stdev: 0 (0.0%)
Runs: 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4 4

Changes To Render Count

There are no entries

Added Scenarios

There are no entries

Removed Scenarios

There are no entries

Generated by 🚫 dangerJS against 6c41d08

@Xiltyn
Copy link
Collaborator Author

Xiltyn commented Feb 7, 2023

I would say, that it might be a good idea to wait until we merge the other CLI init PR so that we could improve on the warning log at the very least. It will have to be move to the new logger service anyway.

I'm happy to readjust as it's super minor in terms of changes, after the former PR will have been merged.

@mdjastrzebski
Copy link
Member

@Xiltyn I think rather than acceping a single file path, we should rather accept a glob that would be somehow passed to Jest for evaluation.

@mdjastrzebski
Copy link
Member

Perhaps, we could use the same --testMatch option as Jest.

@thymikee
Copy link
Member

thymikee commented Feb 8, 2023

Yeah, I'd rather start with proxying testMatch to Jest and when we need to support other test runners some day, we'll rethink this

@Xiltyn
Copy link
Collaborator Author

Xiltyn commented Feb 13, 2023

That's absolutely fair @thymikee @mdjastrzebski. I'll revisit this one and think about a --testMatch based solution instead. Good input!

})
.option('testMatch', {
alias: 'tm',
type: 'string',
Copy link
Member

Choose a reason for hiding this comment

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

It seems that Jest accepts an array of globs, could we support that as well. Yargs seems to support array type.
https://jestjs.io/docs/cli#--testmatch-glob1--globn

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but we're currently using a single string so I went with the current pattern and added an improvement issue to actually properly update and optimise the glob that we're using as default. Please see #363

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would like to make a separate PR to adjust that and also introduce the __perf__ directory as it would reflect the Jest defaults better.

Copy link
Member

@thymikee thymikee Feb 20, 2023

Choose a reason for hiding this comment

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

I'm good with separate PR, let's not block this

@mdjastrzebski
Copy link
Member

@Xiltyn this looks simple and elegant. I've left some minor comments connected with matching Jest options slightly more closely.

@Xiltyn Xiltyn changed the title Added support for running single file test by using an optional --file | -f argument Added support for passing a custom --testMatch option to Jest Feb 20, 2023
@Xiltyn
Copy link
Collaborator Author

Xiltyn commented Feb 20, 2023

@mdjastrzebski @thymikee I updated the PR based on the suggestions. It's mainly a stylistic change since we've already been using the --testMatch option underneath anyway. It should be clearer now though.

@thymikee
Copy link
Member

Needs rebase

@Xiltyn
Copy link
Collaborator Author

Xiltyn commented Feb 20, 2023

Needs rebase

Done 👍

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.

[FEATURE] Ability to run reassure for a single file
3 participants