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.

Sign up for GitHub

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

Jump to bottom

Run CI test on Windows environment #164

Merged
merged 8 commits into from
Jan 24, 2020
Merged

Run CI test on Windows environment #164

merged 8 commits into from
Jan 24, 2020

Conversation

yukukotani
Copy link
Contributor

@yukukotani yukukotani commented Jan 19, 2020 •
edited
Loading

Running CI tests on Windows environment in addition to current Ubuntu environment.

TODO

@yukukotani yukukotani force-pushed the win-support branch 22 times, most recently from 9dab22d to fa12d81 Compare January 20, 2020 01:45
@yukukotani yukukotani closed this Jan 20, 2020
@yukukotani yukukotani reopened this Jan 22, 2020
@yukukotani
Copy link
Contributor Author

Maybe related: jestjs/jest#9413

@FredKSchott
Copy link
Owner

Nice! I tried the fix outlined in that thread (only windows-2016 worked for someone) and I think we're unblocked! We're now seeing test output in the CI job! still failing tho, but at lest now we know why!

'..' is not recognized as an internal or external command, operable program or batch file.

Based on this convo: https://stackoverflow.com/questions/20765337/how-to-fix-is-not-an-internal-or-external-command-error I think this is caused by "TEST": "../../../pkg/dist-node/index.bin.js --clean --include '*.js'".

Based on this convo: https://stackoverflow.com/a/50998274/4682119 I think we can solve it by prefixing our TEST command with node.

@yukukotani
Copy link
Contributor Author

Congrats! But we need to solve failing with windows-latest (windows-2019), because windows-2016 will be removed in this month 😭

@FredKSchott
Copy link
Owner

ugh! Well, I'd still recommend using windows-2016 for now to help us get to a passing suite. Then, we can dig into what's wrong with jest on windows (or just hope that it's a problem that those teams solve themselves :)

@stramel
Copy link
Contributor

stramel commented Jan 23, 2020

I'm fairly certain I have ran into that issue before and adding node before ./ or ../ fixed it for windows.

@FredKSchott
Copy link
Owner

I'm curious now, @monchi hope you don't mind if I push that fix to see if that gets us to green/passing

@FredKSchott
Experimenting with different CI fixes on windows (#173)
2fcb98e 
* experiment with CI variable = env

* strip whitespace in tests, for CI

* fix no optional on ci

* get more output for testing

* logs not working in CI

* strip all whitespace

* fix output

* fix output

* fix output

* fix test file eq output

* skip nomodules test on windows for now

* skip nomodules test on windows for now

* Revert "skip nomodules test on windows for now"

This reverts commit 2fd7806.
@yukukotani
Copy link
Contributor Author

@FredKSchott Of course I don't mind! I'll notice you when this is ready for review.

@yukukotani yukukotani changed the title [WIP] Fix CI error on Windows Run test on Windows environment Jan 24, 2020
@yukukotani yukukotani changed the title Run test on Windows environment Run CI test on Windows environment Jan 24, 2020
@yukukotani yukukotani marked this pull request as ready for review January 24, 2020 02:00
@yukukotani
Copy link
Contributor Author

@FredKSchott Done!

FredKSchott
FredKSchott approved these changes Jan 24, 2020
View reviewed changes
Copy link
Owner

@FredKSchott FredKSchott left a comment

Choose a reason for hiding this comment

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

LGTM!

@FredKSchott FredKSchott merged commit 9926092 into master Jan 24, 2020
@FredKSchott FredKSchott deleted the win-support branch January 24, 2020 06:05
@FredKSchott FredKSchott mentioned this pull request Jan 24, 2020
Closed
@FredKSchott
Copy link
Owner

TODO followups:

  • nomodule: Pika on JS Party?! 🎉 #15
  • windows-2016: If anyone wants to look into this feel free, but otherwise I have a feeling the day this gets removed there will be plenty of issues filed with jest to fix this, and hopefully a bug fix gets merged on their end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FredKSchott FredKSchott FredKSchott approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yukukotani @FredKSchott @stramel @theashraf