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: run windows build on Travis CI #7211

Closed
wants to merge 13 commits into from

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Oct 18, 2018

Summary

🤞

Appveyor is super slow, and on a private account, so we can't cancel redundant builds there. And going from 3 to 2 CIs is only positive.

We might consider adding osx as well, so we have Circle for testing different versions of Node, and Travis to test different OSes.

https://blog.travis-ci.com/2018-10-11-windows-early-release

Test plan

Green!

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

🔥🔥🔥

@thymikee
Copy link
Collaborator

Changleog maybe?

@thymikee
Copy link
Collaborator

You can also remove appveyor bits from testSetupFile.js

.travis.yml Show resolved Hide resolved
@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

You can also remove appveyor bits from testSetupFile.js

Great call!

@thymikee
Copy link
Collaborator

@Daniel15 do you need to remove Jest from your projects or will it be done automatically after we merge this PR? (The builds still trigger even though we removed config)

@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

Might need a repo admin to remove the webhook from settings?


if (global.jasmine && process.env.APPVEYOR_API_URL) {
// Running on AppVeyor, add the custom reporter.
jasmine.getEnv().addReporter(new jasmineReporters.AppVeyorReporter());
Copy link
Contributor

Choose a reason for hiding this comment

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

Too bad Travis doesn't have any concept of test result reporters. This is actually very useful as it pulls out the test results into a separate table view rather than having to look through pages of console output.

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, would be nice if they at least support junit reports...

@Daniel15
Copy link
Contributor

do you need to remove Jest from your projects or will it be done automatically after we merge this PR?

I'm not actually sure. I think an admin of this repo needs to remove the webhook that pings AppVeyor whenever a new PR is opened / commit is made. I'm not an admin so I can't do that.

@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

@SimenB
Copy link
Member Author

SimenB commented Oct 18, 2018

This is unfortunately blocked for now on nodejs/node-gyp#1568 (not node's fault, but easier to link there than the community thing travis uses)

@thymikee thymikee changed the title chore: run windows build on CI chore: run windows build on Travis CI Oct 20, 2018
@thymikee
Copy link
Collaborator

@Daniel15 while this PR is blocked, just one last question (sorry for pinging twice 😅) – is it possible for you to automatically cancel previous builds on AppVeyor so they don't queue up unnecessarily? This is our main source of pain right now when testing on Windows.

Based on their docs it's possible to set "Rolling builds" from settings UI: https://www.appveyor.com/docs/build-configuration/#rolling-builds

@Daniel15
Copy link
Contributor

@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change.

4 similar comments
@Daniel15
Copy link
Contributor

@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change.

@Daniel15
Copy link
Contributor

@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change.

@Daniel15
Copy link
Contributor

@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change.

@Daniel15
Copy link
Contributor

@thymikee No worries. I just enabled the "rolling builds" option. Feel free to ping me again if there's anything else you'd like me to change.

@codecov-io
Copy link

Codecov Report

Merging #7211 into master will decrease coverage by 0.76%.
The diff coverage is 91.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7211      +/-   ##
==========================================
- Coverage   67.25%   66.49%   -0.77%     
==========================================
  Files         248      237      -11     
  Lines        9641     9302     -339     
  Branches        3        4       +1     
==========================================
- Hits         6484     6185     -299     
+ Misses       3156     3116      -40     
  Partials        1        1
Impacted Files Coverage Δ
packages/jest-config/src/ValidConfig.js 100% <ø> (ø) ⬆️
packages/jest-config/src/Defaults.js 100% <ø> (ø) ⬆️
...ackages/jest-cli/src/reporters/summary_reporter.js 70.49% <100%> (ø) ⬆️
packages/expect/src/matchers.js 97.48% <100%> (+0.03%) ⬆️
packages/jest-runtime/src/should_instrument.js 100% <100%> (ø) ⬆️
packages/jest-config/src/normalize.js 85.41% <100%> (+0.31%) ⬆️
packages/jest-cli/src/SearchSource.js 57.69% <100%> (-0.54%) ⬇️
packages/expect/src/utils.js 98.16% <100%> (ø) ⬆️
packages/jest-haste-map/src/index.js 92.05% <100%> (+0.02%) ⬆️
packages/jest-cli/src/watch.js 81.14% <33.33%> (-0.84%) ⬇️

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 814c929...b9fb717. Read the comment docs.

@jestjs jestjs deleted a comment from Daniel15 Oct 22, 2018
@SimenB
Copy link
Member Author

SimenB commented Oct 22, 2018

Ok, managed to install python and get it added to $PATH. Next issue is that we seem to have lost colors (supports-colors probably returns false because of reasons), so many tests are failing...

EDIT: Also, the windows build doesn't seem to exit properly even though our script completes. It just hangs

@SimenB
Copy link
Member Author

SimenB commented Nov 18, 2018

Should not be hanging after the last commit, but we are still missing colors...

What about using azure pipelines? Like prettier/prettier#5410

@kaylangan
Copy link
Contributor

What about using azure pipelines? Like prettier/prettier#5410

Azure Pipelines has support for junit test reporting (as you might've seen prettier use). You can also build on Mac and Linux in addition to Windows, so you could use Azure Pipelines to test OS's and Circle for versions of Node. That said, if you're inclined to move everything to one CI service, Azure Pipelines can test across Node versions too, and we'd be happy to give you additional concurrency. Let me know if you have any questions or suggestions. I'm a Program Manager on Azure Pipelines and happy to help!

@Daniel15
Copy link
Contributor

We've been working on moving the Yarn build to Azure Pipelines too :) Seems like a good service.

@rickhanlonii
Copy link
Member

I'm +1 for moving to azure instead 😸

@SimenB
Copy link
Member Author

SimenB commented Dec 25, 2018

Yeah, my vote is for Azure pipelines as well. We don't run Travis on master since colors are messed up, which means we'd not be running windows on master, which I don't think is acceptable.

@kaylangan What's the easiest way forward? I'm not an admin on this repo, is that needed?

you could use Azure Pipelines to test OS's and Circle for versions of Node

That sounds perfect, at least to start with 🙂

@kaylangan
Copy link
Contributor

@SimenB I've created #7556 that will add initial support for Azure Pipelines. You will need repo permissions to set up the Azure Pipelines GitHub app.

@SimenB
Copy link
Member Author

SimenB commented Jan 14, 2019

let's go for #7556

@SimenB SimenB closed this Jan 14, 2019
@SimenB SimenB deleted the windows-travis branch January 14, 2019 21:03
@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.

None yet

7 participants