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

Skip puppeteer download on unit test runs, fix windows tests, polish pipeline #2541

Merged
merged 16 commits into from
Nov 6, 2019

Conversation

Tyriar
Copy link
Member

@Tyriar Tyriar commented Nov 5, 2019

Fixes #2539
Fixes #2542
Fixes #2543

@Tyriar
Copy link
Member Author

Tyriar commented Nov 5, 2019

Skipping puppeteer download seems to save about 15 seconds on Linux, let's see if caching yarn cache works.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 6, 2019

@jerch
Copy link
Member

jerch commented Nov 6, 2019

@Tyriar So the difference is basically a minute for a single pipeline?

@Tyriar
Copy link
Member Author

Tyriar commented Nov 6, 2019

@jerch yeah, integration tests are mostly unchanged at but I'll see if I can reduce that a bit too. So right now we'll get faster compile/unit test feedback.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 6, 2019

Caching node_modules for integration tests didn't seem to work, also it ended up as 1gb so it was going to be slower anyway if it did.

@Tyriar Tyriar changed the title Skip puppeteer download on unit test runs Skip puppeteer download on unit test runs, fix windows tests, polish pipeline Nov 6, 2019
@Tyriar
Copy link
Member Author

Tyriar commented Nov 6, 2019

Final results gives faster compile and unit test builds to let us iterate faster on PRs:

  Before After
Linux 2m 58s 1m 42s
macOS 1m 58s 1m 50s*
Windows 3m 23s** 2m 3s

* The actual run took 2m 23s but that's because cache restore took an abnormally long time just this run, number was adjusted for typical runtime
** This is the approx value, Windows wasn't running tests before so it's the actual number 2m 55s plus 28s for unit tests

Integration tests are basically the same.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 6, 2019

This passed fully on a rebuild

@Tyriar Tyriar merged commit 16409da into xtermjs:master Nov 6, 2019
@Tyriar Tyriar deleted the 2539 branch November 6, 2019 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants