-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Add AppVeyor configuration #1465
Conversation
This is supposed to be stacked on top of #1463, but I forgot that Github is not Phabricator and doesn't support basic features such as that. The only change in this pull request is supposed to be the |
Current coverage is 90.31% (diff: 100%)
|
are you interested in fixing up all the tests to work on windows? |
Sure, I can take a look when I get a chance. |
4b486dd
to
7650ac1
Compare
@cpojer - What do you think of this approach for now? I simply make Windows builds skip all the tests that fail on Windows. It's not ideal and I do plan on going through and fixing the tests incrementally, but at least it makes a Windows build actually possible and will still help catch some regressions. It works! https://ci.appveyor.com/project/Daniel15/jest/build/7/job/apivm49w0c7nm4hc |
@@ -0,0 +1,22 @@ | |||
environment: | |||
matrix: | |||
- nodejs_version: "4" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's just not support node 4 on windows. I don't think we'll be able to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any issues with Node 4 that would cause it to break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No but I'm deciding right now that the only node version we support on Windows will be the latest.
Can we instead just dump the skipOnWindows file in |
In testSetupFile.js you could also do: jest.mock('skipOnWindows', () => {
return implementation.
}, {virtual: true}); |
I'm having some trouble getting Jest running on my work laptop, so I'll do those changes tonight on my home PC instead 👍 |
Thank you @Daniel15! One problem we often run into is that Jest has an issue with test paths and can't find any more tests. The exit code in this case is 0, so it says everything is fine. Can we add an additional check to appveyor that makes sure at least one test was run? |
We have an integration test for that, however that uses Jest too, so if Jest breaks like this, we won't actually know :D |
Hmm, that's tricky... I feel like the exit code should actually be non-zero Sent from my phone. On Sep 3, 2016 1:53 AM, "Christoph Pojer" [email protected] wrote:
|
I'm happy to accept a PR that adds this flag, yes. I'm worried about this having a bad impact on www where our test infra might try to run something that isn't actually a test; we don't want to fail and require changes to the test infra there. |
Add AppVeyor configuration
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. |
The build doesn't actually work properly yet (a bunch of tests fail: https://ci.appveyor.com/project/Daniel15/jest/build/1.0.5/job/wjnpa9thok6twp5y) but it's worth at least starting to work on this. Once all the broken tests are fixed on Windows builds, having a continuous integration setup like this will prevent it from breaking again in the future.
Closes #1192