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

[Merged by Bors] - Cleanup and speed-up CI #2376

Closed
wants to merge 9 commits into from
Closed

[Merged by Bors] - Cleanup and speed-up CI #2376

wants to merge 9 commits into from

Conversation

RageKnify
Copy link
Member

@RageKnify RageKnify commented Oct 24, 2022

Currently we run 7 different jobs:

  • tests linux
  • tests macos
  • tests windows
  • rustfmt
  • clippy
  • examples
  • documentation

With this change I reduced them to 4 and hopefully sped them up.

The total execution time is limited by the tests, especially linux that calculates coverage. Having separate jobs for clippy and rustfmt (which take a very small amount of time) is a waste of energy.
With this PR:
Introduced a new cargo profile, ci, that should create smaller sized binaries and reduce the our cache usage.
I changed the test runner for macos and windows to nextest, which should be faster and is specifically designed for CI.
I merged all smaller tasks in a single job, misc, the steps clearly identify what is being tested so it shouldn't affect clarity.
Switched to using the rust-cache GH action, this simplifies our work by no longer having to worry about which directories to cache, rust-cache handles all that for us.

The bors task should also be modified, I'll get to it as soon as I have time. I believe it should be possible for us to have a single workflow described and have it both be the normal CI and the bors test.

@github-actions
Copy link

github-actions bot commented Oct 24, 2022

Test262 conformance changes

Test result main count PR count difference
Total 93,789 93,789 0
Passed 69,150 69,150 0
Ignored 18,352 18,352 0
Failed 6,287 6,287 0
Panics 0 0 0
Conformance 73.73% 73.73% 0.00%

@RageKnify RageKnify added the Internal Category for changelog label Oct 24, 2022
@codecov
Copy link

codecov bot commented Oct 24, 2022

Codecov Report

Merging #2376 (ac04a60) into main (946a4dd) will decrease coverage by 0.13%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2376      +/-   ##
==========================================
- Coverage   40.03%   39.89%   -0.14%     
==========================================
  Files         304      304              
  Lines       23401    23332      -69     
==========================================
- Hits         9368     9308      -60     
+ Misses      14033    14024       -9     
Impacted Files Coverage Δ
boa_engine/src/syntax/ast/expression/yield.rs 5.88% <0.00%> (-11.77%) ⬇️
boa_engine/src/builtins/promise/promise_job.rs 30.30% <0.00%> (-5.19%) ⬇️
boa_engine/src/syntax/ast/statement/switch/mod.rs 38.46% <0.00%> (-5.13%) ⬇️
boa_engine/src/vm/opcode/pop/mod.rs 66.66% <0.00%> (-4.77%) ⬇️
boa_engine/src/object/internal_methods/mod.rs 64.62% <0.00%> (-4.37%) ⬇️
boa_engine/src/vm/opcode/rest_parameter/mod.rs 82.35% <0.00%> (-4.32%) ⬇️
...ngine/src/syntax/ast/expression/tagged_template.rs 36.00% <0.00%> (-4.00%) ⬇️
boa_engine/src/syntax/ast/statement/try/mod.rs 43.63% <0.00%> (-3.64%) ⬇️
boa_engine/src/syntax/ast/declaration/mod.rs 75.75% <0.00%> (-3.04%) ⬇️
boa_engine/src/builtins/mod.rs 10.29% <0.00%> (-2.95%) ⬇️
... and 86 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Looks good, just a small comment typo :)

Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Thanks for the enhancement!

Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks good!

@jedel1043

This comment was marked as outdated.

@bors

This comment was marked as outdated.

@jedel1043 jedel1043 added this to the v0.17.0 milestone Oct 24, 2022
@jedel1043 jedel1043 added the github_actions Pull requests that update Github_actions code label Oct 24, 2022
@jedel1043

This comment was marked as outdated.

@bors

This comment was marked as outdated.

@jedel1043
Copy link
Member

Ah, you forgot to change the bors.toml status to the new names.

@RageKnify
Copy link
Member Author

Yeah, I still want to modify the bors action, I'll get to it today.

@RageKnify RageKnify changed the title chore: Cleanup and speed-up CI Cleanup and speed-up CI Oct 25, 2022
This introduces multiple changes to our CI setup
- Switch cache action to rust-cache, this handles which directories to
  cache for us
- Switch to the nextest[2] test runner for Windows and MacOS
- Join the smaller jobs into one bigger misc job that handles clippy,
rustfmt, documentation and examples. This should still be faster than
the linux job and should save some resources overall

[1] - https://github.com/Swatinem/rust-cache
[2] - https://nexte.st/
tarpaulin is very slow already, this should balance things out
This will make them have separate cache keys, meaning they don't
share their target directory and each one stores and loads a smaller
archive from the cache.
Simplified the name of the jobs in rust.yml
Removed bors.yml, it was just copying rust.yml, instead this adds
the staging and trying branches to the list of branches where the ci
should run
@RageKnify RageKnify added the hacktoberfest-accepted PR accepted for Hacktoberfest label Oct 25, 2022
@RageKnify
Copy link
Member Author

bors r+

bors bot pushed a commit that referenced this pull request Oct 25, 2022
Currently we run 7 different jobs:
- tests linux
- tests macos
- tests windows
- rustfmt
- clippy
- examples
- documentation

With this change I reduced them to 4 and hopefully sped them up.

The total execution time is limited by the tests, especially linux that calculates coverage. Having separate jobs for clippy and rustfmt (which take a very small amount of time) is a waste of energy.
With this PR:
Introduced a new cargo profile, `ci`, that should create smaller sized binaries and reduce the our cache usage.
I changed the test runner for macos and windows to [nextest](https://nexte.st/), which should be faster and is specifically designed for CI.
I merged all smaller tasks in a single job, misc, the steps clearly identify what is being tested so it shouldn't affect clarity.
Switched to using the [rust-cache](https://github.com/Swatinem/rust-cache) GH action, this simplifies our work by no longer having to worry about which directories to cache, rust-cache handles all that for us.

~~The bors task should also be modified, I'll get to it as soon as I have time. I believe it should be possible for us to have a single workflow described and have it both be the normal CI and the bors test.~~
@bors
Copy link

bors bot commented Oct 25, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Cleanup and speed-up CI [Merged by Bors] - Cleanup and speed-up CI Oct 25, 2022
@bors bors bot closed this Oct 25, 2022
@bors bors bot deleted the clean-ci branch October 25, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github_actions Pull requests that update Github_actions code hacktoberfest-accepted PR accepted for Hacktoberfest Internal Category for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants