Speedup ci #1556
Merged
Speedup ci #1556
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@Byron it shows that this PR makes ci.yml 5m faster |
Byron
approved these changes
Aug 25, 2024
There was a problem hiding this comment.
This is a very impressive outcome, thanks so much!
I am also surprised that cargo nextest manages to do the same faster, but maybe it's just the removal of clear-target (which exists there for reasons lost to time) which so much.
In any case, I prefer the output of cargo nextest and wasn't aware it can be used as cargo test stand-in - it's very neat, indeed!
It could be caching, but nextest is indeed faster if you have lots of tests and more than 1 cores, I checked the log it does run tests for some crates faster by running them in parallel by spawning one process for each tests. It might be more robust if your tests involve global variables or potential UBs, running in separate processes protects against that. |
It's not a full drop-in, it can't run doc-test right now, you'd need However I think you don't have the habit of using doc-test so often as most of the valuable tests is probably in |
|
P.S. nextest also integrates with llvm-cov, miri, criterion and cargo-mutants |
|
https://github.com/nextest-rs/reuse-build-partition-example/blob/main/.github/workflows/ci.yml And it can:
|
|
Thanks again for the heads-up! |
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Aug 26, 2024
This runs `cargo nextest ...` and `cargo test --doc` in separate steps in the `test-fast` job, so that the job fails when `cargo nextest ...` fails. Otherwise, with `pwsh` on Windows, test failures (other than doctests) are masked. Background: Since 89a0567 (GitoxideLabs#1556), doctests are run on all three major platforms, and not only on the full test job done on Ubuntu. But the way this was done relied on a script failing as soon as (or, at least, whenever) any command in the script failed. That works on Ubuntu and macOS, where a `bash` shell is used by default, with `-e` passed. But on Windows, GitHub Actions uses `pwsh` as the default shell. `pwsh` is not run in a way that causes it to stop at the first failing command. So, on Windows, when the `cargo nextest` command failed but the `cargo test --doc` command that followed it in the same script step passed, the step passed, thus allowing the job and workflow to pass. This was observed in GitoxideLabs#1429 after a rebase (see comments). Note that this is not related to the increased use of `nextest`. While that was also done in GitoxideLabs#1556, it did not affect the `test-fast` job where the bug was introduced, which was already using `nextest`. This fixes the problem by putting the two commands in separate steps. This is simpler than doing anything in PowerShell to make the script stop, such as using `&&` or attempting to produce `-e`-like behavior. Another option could be to use `bash` as the shell, which is a Git Bash environment suitable for running the tests. The reason I didn't do that is that I think it is valuable to see the results when the tests are run from a PowerShell environment. In particular, continuing to use PowerShell here should help in investigating GitoxideLabs#1359 (and shows that the claim I made is overly strong, since CI on Windows with `pwsh` not itself started from a Unix-style shell is not "Git Bash or a similar environment").
This was referenced Aug 26, 2024
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 6, 2024
The `test-fast` job has been intended to run doctests since 89a0567 (GitoxideLabs#1556). But because there are no doctests in the top-level project and neither `--workspace` nor its `--all` alias were passed, the effect has been: Compiling ... Finished `test` profile [unoptimized + debuginfo] target(s) in 28.55s Doc-tests gitoxide running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s (Where ... stands for details in various "Compiling lines. That output is copied from https://github.com/GitoxideLabs/gitoxide/actions/runs/11690609999/job/32555922512#step:9:70 though that log will eventually become available, and only the build time changes.) Note that zero tests are run and the status reports zero of each possible kind of result. There are (at least currently) no doctests in the top-level package, and `--workspace` is not implied. This adds `--workspace` to the command that runs the doctests, so it will collect and run doctests throughout the entire workspace. For now, this is not done on the corresponding command in the `unit-tests` rule in `justfile`; it may make sense to do that, but if it is done, then this step should probably be omitted on the `ubuntu-latest` run of `test-fast` since the CI job that runs `just unit-tests` is `test` which itself runs on `ubuntu-latest`. (The changes in GitoxideLabs#1556 were revised in GitoxideLabs#1559, but that only fixed a problem with reporting results from non-doctest tests. I had not noticed the problem of not running any doctests at that time.)
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 6, 2024
The `test-fast` job has been intended to run doctests since 89a0567 (GitoxideLabs#1556). But because there are no doctests in the top-level project and neither `--workspace` nor its `--all` alias were passed, the effect has been: Compiling ... Finished `test` profile [unoptimized + debuginfo] target(s) in 28.55s Doc-tests gitoxide running 0 tests test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s (Where ... stands for details in various "Compiling lines. That output is copied from https://github.com/GitoxideLabs/gitoxide/actions/runs/11690609999/job/32555922512#step:9:70 though that log will eventually become unavailable, and only the build time changes.) Note that zero tests are run and the status reports zero of each possible kind of result. There are (at least currently) no doctests in the top-level package, and `--workspace` is not implied. This adds `--workspace` to the command that runs the doctests, so it will collect and run doctests throughout the entire workspace. For now, this is not done on the corresponding command in the `unit-tests` rule in `justfile`; it may make sense to do that, but if it is done, then this step should probably be omitted on the `ubuntu-latest` run of `test-fast` since the CI job that runs `just unit-tests` is `test` which itself runs on `ubuntu-latest`. (The changes in GitoxideLabs#1556 were revised in GitoxideLabs#1559, but that only fixed a problem with reporting results from non-doctest tests. I had not noticed the problem of not running any doctests at that time.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
target