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

add windows to the azure pipeline #8018

Merged
merged 1 commit into from Sep 13, 2018
Merged

add windows to the azure pipeline #8018

merged 1 commit into from Sep 13, 2018

Conversation

sokra
Copy link
Member

@sokra sokra commented Sep 12, 2018

What kind of change does this PR introduce?
CI

Did you add tests for your changes?
N/A

Does this PR introduce a breaking change?
N/A

What needs to be documented once your changes are merged?
nothing

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@montogeek
Copy link
Member

It is not possible to see the build?

@sokra
Copy link
Member Author

sokra commented Sep 12, 2018

Blocked by jestjs/jest#6965 because of a memory leak

@sokra
Copy link
Member Author

sokra commented Sep 12, 2018

@montogeek
Copy link
Member

I can see that page, but it is not possible from https://github.com/webpack/webpack/runs/16950372 links.

@sokra
Copy link
Member Author

sokra commented Sep 12, 2018

You can click on the View more details on Azure Pipelines link on this page

@montogeek
Copy link
Member

It returns this page
image.

Just wanted to see how it looks like. Thanks for the link.

@sokra
Copy link
Member Author

sokra commented Sep 12, 2018

hmm... The project is public:

image

@sokra
Copy link
Member Author

sokra commented Sep 12, 2018

hmm weird that one even need to authenticate to view the page.

@davidstaheli
Copy link
Contributor

Hi webpack friends. I'm from the Azure Pipelines team. I'm so sorry for the trouble with this. A bug crept in causing those URLs to require sign in. We should have it hot-fixed in production tomorrow. From that point on, builds will produce URLs that allow anonymous access. Sorry again for the trouble.

@TheLarkInn
Copy link
Member

Thanks @davidstaheli!!!

@sokra
Copy link
Member Author

sokra commented Sep 13, 2018

Hi @davidstaheli thanks for joining in.

Here a few updates regarding the windows azure status:

It seem like some of the problems on windows are caused by a memory leak in the test suite. I already filed a PR regarding this, which seem to fixes the problem locally for me (jestjs/jest#6965). I haven't tried it on azure. I wanted to wait until the PR is merged and released. This probably also changes the performance on other platforms.

As workaround I added --maxWorkers=2 like in appveyor and travis, this seem to work. It also speeds up the builds a bit...

@sokra
Copy link
Member Author

sokra commented Sep 13, 2018

@davidstaheli Travis and appveyor do have "caches" usable by the builds (https://docs.travis-ci.com/user/caching/). Does something similar exists for azure? We currently use these caches to persist the yarn cache (for dependencies) and the jest transformation cache (for the source transformation for code coverage).

@sokra
Copy link
Member Author

sokra commented Sep 13, 2018

Ok here are some results:

On travis the watching test cases are disabled (because they tend to fail randomly), I accidentally left them enabled in azure and they doesn't seem to fail. I will keep them enabled, if they don't cause any problems. I will watch this...

So assuming this continue to work => We can now run these tests in CI 👍


Appveyor has a free limit of 1 parallel build, Travis has a free limit of ~8 (unsure) parallel builds, Azure has a free limit of 10 parallel builds.

Currently appveyor runs 3 build steps: unit, integration in node.js 10, integration in node.js 6. They run in series because of the parallel limit. The total time for this is ~22minutes.

Azure runs with 2 + 9 build steps: First it runs basic+unit tests and linting, after these succeeded it run the integration tests in all combinations of (win, linux, macos)x(node 6, 8, 10). The total time for this is ~9.5minutes (with watch tests ~11minutes)

Integration test appveyor travis azure
windows node 10 9:00 - 6:27
windows node 8 - - 6:39
windows node 6 10:29 - 7:35
linux node 10 - 5:11 4:48
linux node 8 - 5:13 4:41
linux node 6 - 6:31 5:22
macos node 10 - 6:50 3:53
macos node 8 - - 4:10
macos node 6 - - 5:18

Note for better comparision watch tests has been excluded from this build, so all CIs run the same amount of tests.

Machines on azure seem to be faster in general. Especially macOS machines.

=> When replacing appveyor with azure CI builds will take about half as long. 👍

Currently we don't use any caching on azure. This seem to be an additional opportunity to speed up builds.

Here is a summary of the individual timings:

Step travis azure
restore cache 8.8s -
install node 2.5s 5.1s
install yarn preinstalled 4.3s
yarn install 8.7s 27.2s
store cache 14.3s -

Note that the cache also includes the jest transformation cache for code coverage. So it's unclear if the caching is beneficial for yarn install or not. Maybe we should investigate here...

@sokra sokra merged commit c79c1de into master Sep 13, 2018
@sokra sokra deleted the ci/azure-windows branch September 13, 2018 09:42
@sendilkumarn
Copy link
Member

@sokra thats quite a stat 👍🎉 Thanks for writing this down

@davidstaheli
Copy link
Contributor

davidstaheli commented Sep 13, 2018

Unfortunately Microsoft-hosted builds don't yet support caching between builds. 😞 We're working on that and other ways to make builds faster. Please let me and @vtbassmatt know if you ever need more than the 10 parallel jobs, or if we can help with anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants