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

Fix yarn test on windows #6534

Merged
merged 6 commits into from
Jul 3, 2018
Merged

Conversation

hron
Copy link
Contributor

@hron hron commented Jun 24, 2018

Summary

Some of the tests fail on Windows when they are run with yarn test. Several fails are due to concurrency problem, one is about relying on LF as EOF in one of the json file.

Test plan

Before

image

After

image

.gitattributes Outdated
@@ -0,0 +1 @@
packages/jest-config/src/__tests__/jest-preset.json eol=lf
Copy link
Member

Choose a reason for hiding this comment

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

can we fix the test instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To do this I guess I need to replace CRLF with LF in runtime here (https://github.com/facebook/jest/blob/master/packages/jest-config/src/__tests__/normalize.test.js#L936-L950):

    jest.doMock('/node_modules/react-native/jest-preset.json', () =>
      require.requireActual('./jest-preset.json'),
    );

I'm not sure how to do this properly and it doesn't seem like a good solution to me. Here is the failing output for the details:

Click to update snapshot for 'throws when preset is invalid' test

Error: expect(value).toMatchSnapshot()

Received value does not match stored snapshot "preset throws when preset is invalid 1".

- Snapshot
+ Received

  "<red><bold><bold>● <bold>Validation Error</>:</>
  <red></>
  <red>  Preset <bold>react-native</> is invalid:</>
- <red>  Unexpected token } in JSON at position 104</>
+ <red>  Unexpected token } in JSON at position 110</>
  <red></>
  <red>  <bold>Configuration Documentation:</></>
  <red>  https://facebook.github.io/jest/docs/configuration.html</>
  <red></>"
    at Object.test (E:\src\jest\packages\jest-config\src\__tests__\normalize.test.js:949:8)
    at Object.asyncFn (E:\src\jest\packages\jest-jasmine2\build\jasmine_async.js:108:37)
    at resolve (E:\src\jest\packages\jest-jasmine2\build\queue_runner.js:56:12)
    at new Promise (<anonymous>)
    at mapper (E:\src\jest\packages\jest-jasmine2\build\queue_runner.js:43:19)
    at promise.then (E:\src\jest\packages\jest-jasmine2\build\queue_runner.js:87:41)
    at <anonymous>

@hron
Copy link
Contributor Author

hron commented Jul 1, 2018

@SimenB what about using .toThrowError here instead of trying to match a snapshot that depends on CRLF vs LF?

@SimenB
Copy link
Member

SimenB commented Jul 1, 2018

That sounds good to me! Mind removing the obsolete snapshot and rebasing?

@hron hron force-pushed the fix-yarn-test-on-windows branch from 44202b2 to 8f8effc Compare July 1, 2018 17:33
@hron
Copy link
Contributor Author

hron commented Jul 1, 2018

The obsolete snapshot has been removed. I hope CI will pass now.

@codecov-io
Copy link

Codecov Report

Merging #6534 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6534   +/-   ##
=======================================
  Coverage   63.73%   63.73%           
=======================================
  Files         235      235           
  Lines        8931     8931           
  Branches        4        4           
=======================================
  Hits         5692     5692           
  Misses       3238     3238           
  Partials        1        1

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 d6e0a1b...50f99a7. Read the comment docs.

@SimenB SimenB merged commit 9bb8909 into jestjs:master Jul 3, 2018
schalkneethling referenced this pull request in mdn/interactive-examples Jul 6, 2018
This Pull Request renovates the package group "jest monorepo".


-   [jest-environment-node](https://github.com/facebook/jest) (`devDependencies`): from `23.2.0` to `23.3.0`
-   [jest](https://github.com/facebook/jest) (`devDependencies`): from `23.2.0` to `23.3.0`

# Release Notes
<details>
<summary>facebook/jest</summary>

### [`v23.3.0`](https://github.com/facebook/jest/blob/master/CHANGELOG.md#&#8203;2330)
[Compare Source](jestjs/jest@v23.2.0...v23.3.0)
##### Features

- `[jest-cli]` Allow watch plugin to be configured ([#&#8203;6603](`https://github.com/facebook/jest/pull/6603`))
- `[jest-snapshot]` Introduce `toMatchInlineSnapshot` and `toThrowErrorMatchingInlineSnapshot` matchers ([#&#8203;6380](`https://github.com/facebook/jest/pull/6380`))
##### Fixes

- `[jest-regex-util]` Improve handling already escaped path separators on Windows ([#&#8203;6523](`https://github.com/facebook/jest/pull/6523`))
- `[jest-cli]` Fix `testNamePattern` value with interactive snapshots ([#&#8203;6579](`https://github.com/facebook/jest/pull/6579`))
- `[jest-cli]` Fix enter to interrupt watch mode ([#&#8203;6601](`https://github.com/facebook/jest/pull/6601`))
##### Chore & Maintenance

- `[website]` Switch domain to https://jestjs.io ([#&#8203;6549](`https://github.com/facebook/jest/pull/6549`))
- `[tests]` Improve stability of `yarn test` on Windows ([#&#8203;6534](`https://github.com/facebook/jest/pull/6534`))
- `[*]` Transpile object shorthand into Node 4 compatible syntax ([#&#8203;6582](`https://github.com/facebook/jest/pull/6582`))
- `[*]` Update all legacy links to jestjs.io ([#&#8203;6622](`https://github.com/facebook/jest/pull/6622`))
- `[docs]` Add docs for 23.1, 23.2, and 23.3 ([#&#8203;6623](`https://github.com/facebook/jest/pull/6623`))
- `[website]` Only test/deploy website if relevant files are changed ([#&#8203;6626](`https://github.com/facebook/jest/pull/6626`))
- `[docs]` Describe behavior of `resetModules` option when set to `false` ([#&#8203;6641](`https://github.com/facebook/jest/pull/6641`))

---


</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
@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 May 12, 2021
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.

5 participants