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

feat: add --platform flag to override haste.defaultPlatform #11764

Closed
wants to merge 3 commits into from

Conversation

tido64
Copy link

@tido64 tido64 commented Aug 18, 2021

Summary

Adds a new flag, --platform, to override haste.defaultPlatform option. This is useful for tests that need to be run against multiple platform targets. Previously, you would need separate Jest configs for each platform and set haste.defaultPlatform accordingly. With this flag, you can have one config and instead specify --platform ios instead.

Test plan

Normally, running react-native tests without a haste configuration will fail:

test-app % yarn jest
 FAIL  test/App.test.tsx
  ● Test suite failed to run

    Cannot find module '../Libraries/Image/Image' from '../../node_modules/react-native/jest/setup.js'

      at Resolver.resolveModule (../../node_modules/jest-resolve/build/resolver.js:311:11)
      at Object.<anonymous> (../../node_modules/react-native/jest/setup.js:58:4)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        1.028 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

With this change, you can now specify the platform from the command line:

% yarn jest --platform ios
 PASS  test/App.test.tsx
  ✓ renders correctly (239 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        0.747 s
Ran all test suites.
✨  Done in 2.33s.

@codecov-commenter
Copy link

Codecov Report

Merging #11764 (cdbf2e3) into master (8d33ba1) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11764      +/-   ##
==========================================
- Coverage   69.00%   68.98%   -0.02%     
==========================================
  Files         312      312              
  Lines       16339    16345       +6     
  Branches     4736     4740       +4     
==========================================
+ Hits        11275    11276       +1     
- Misses       5036     5041       +5     
  Partials       28       28              
Impacted Files Coverage Δ
packages/jest-config/src/setFromArgv.ts 75.60% <0.00%> (-12.97%) ⬇️
packages/expect/src/utils.ts 96.13% <0.00%> (+0.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d33ba1...cdbf2e3. Read the comment docs.

Copy link
Contributor

@sigveio sigveio left a comment

Choose a reason for hiding this comment

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

A changelog entry seems to be missing 🙂

@tido64
Copy link
Author

tido64 commented Aug 18, 2021

A changelog entry seems to be missing 🙂

Thanks, I thought I pushed it already 😄

@tido64 tido64 force-pushed the tido/add-platform-flag branch from ef1211d to f58be19 Compare August 30, 2021 07:11
@tido64
Copy link
Author

tido64 commented Aug 30, 2021

@SimenB: I would really appreciate it if you reviewed this PR and let me know how I can improve it.

@tido64
Copy link
Author

tido64 commented Sep 6, 2021

@sigveio, @SimenB: Please let me know if there is someone else I can ping to help review this PR. Thanks 😄

@SimenB
Copy link
Member

SimenB commented Sep 29, 2021

Hi! Sorry about the delay.


Is there any reason you cannot use the existent --haste flag? I'd really love to not add any more flags 😅

@tido64
Copy link
Author

tido64 commented Sep 29, 2021

Is there any reason you cannot use the existent --haste flag? I'd really love to not add any more flags 😅

Ah, it didn't occur to me that it exists (I combed through CLI options looking for it), even though I touched the code 😛

Does that mean that we could do something like --haste '{ defaultPlatform: "ios" }'? Compared to --platform ios, I think the latter is a lot more user-friendly, don't you think? It's not exactly what comes to mind if anyone asked me how to target a specific platform.

@mrazauskas
Copy link
Contributor

@tido64 --haste='{"defaultPlatform": "ios"}' seems to be working. The value must be JSON string.

@SimenB
Copy link
Member

SimenB commented Sep 29, 2021

I agree it's more user friendly, but a metric ton of options isn't very nice either... Also, passing both haste and this new flag would conflict, and which case should win isn't obvious to the end user I think regardless of which way we land it.

(And yeah, there's a bunch of CLI args we haven't documented - jest --help lists them all tho)

@github-actions
Copy link

This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Sep 29, 2022
@tido64 tido64 closed this Sep 29, 2022
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2022
@tido64 tido64 deleted the tido/add-platform-flag branch October 31, 2022 08:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants