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

chore: windows ci #691

Closed
wants to merge 7 commits into from
Closed

chore: windows ci #691

wants to merge 7 commits into from

Conversation

styfle
Copy link
Member

@styfle styfle commented Apr 11, 2021

Related to #689

@guybedford
Copy link
Contributor

Perhaps try updating the version of node-gyp here?

@styfle
Copy link
Member Author

styfle commented Apr 11, 2021

@guybedford Bumping node-gyp helped but now jest doesn't run. Seems like we need to expose GC and make it windows compatible in order to run the tests.

@@ -15,8 +15,8 @@
"build": "node scripts/build",
"build-test-binary": "cd test/binary && node-gyp rebuild && cp build/Release/hello.node ../integration/hello.node",
"codecov": "codecov",
"test": "node --expose-gc --max_old_space_size=3072 node_modules/.bin/jest",
"test-coverage": "node --expose-gc --max_old_space_size=3072 node_modules/.bin/jest --coverage --globals \"{\\\"coverage\\\":true}\" && codecov",
Copy link
Contributor

Choose a reason for hiding this comment

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

--max_old_space_size is necessary here.

Copy link
Member Author

@styfle styfle Apr 12, 2021

Choose a reason for hiding this comment

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

Thanks! I added cross-env and that got past the memory issue, but now jest doesn't find any tests on Windows 🤔

@guybedford
Copy link
Contributor

Seems like it might have something to do with the first ? in the configuration matcher - https://github.com/vercel/ncc/blob/master/jest.config.js#L5.

CI reports: testMatch: D:/a/ncc/ncc/test\?(*.)+(spec|test).js?(x) - 0 matches

While the matcher configuration has: testMatch: ["<rootDir>/test/?(*.)+(spec|test).js?(x)"]

Try simplifying the rule to just:

testMatch: ["<rootDir>/test/*.test.js"]

@codecov-io
Copy link

Codecov Report

Merging #691 (a4a39e6) into master (4d42c21) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #691   +/-   ##
=======================================
  Coverage   73.41%   73.41%           
=======================================
  Files          13       13           
  Lines         474      474           
=======================================
  Hits          348      348           
  Misses        126      126           

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 4d42c21...a4a39e6. Read the comment docs.

@styfle
Copy link
Member Author

styfle commented Apr 13, 2021

@guybedford Good call!

Looks like some of the tests are failing because windows emits \r\n instead of the expected \n so thats not a big issue.

However, there are a couple that look like actual bugs: https://github.com/vercel/ncc/pull/691/checks?check_run_id=2333639573#step:9:318

@styfle
Copy link
Member Author

styfle commented Jun 1, 2022

Fixed in #896

@styfle styfle closed this Jun 1, 2022
@styfle styfle deleted the windows-ci branch June 1, 2022 18:44
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.

3 participants