More comprehensive CI test matrix #2951
Comments
thomaseizinger
added
difficulty:easy
help wanted
getting-started
|
Just a general question on MSRV: I never quite understood why that was a thing. Are there any real reasons not to use the latest version of Rust stable? |
Yes, multiple reasons in fact:
Even ignoring all of that, depending on newer features will cause compilation with an older compiler to break. According to semver, breaking changes need to come with an appropriate bump. If we ignore this part, we are violating semver which can cause major headaches because software like Thus, even if we don't have a strict MSRV policy, our CI should tell us if we are making a breaking change because we are bumping the MSRV :) |
|
Those are some very good reasons. Thanks a lot for explaining! I will take a spin at implanting this issue if I've got the time then |
|
I didn't find any GitHub Action that did what I was looking for. Gonna write my own. |
There should be no need for a GitHub action really. We just need a matrix :) See https://github.com/Restioson/xtra/blob/master/.github/workflows/ci.yml for example. |
|
Oh, than there has been a misunderstanding. I thought you wanted an Action to tell you what the MSRV for a crate is. If it's just a very big matrix were we test a lot of versions, that is easier. |
You can get the MSRV from the manifest with some shell scripting: https://github.com/testcontainers/testcontainers-rs/blob/d7534a56d4313c8c707a1975d70874eff8a67d69/.github/workflows/ci.yml#L11 |
|
Adding a CI for that was a very good idea 😅, as it turns out we actually don't compile on Rust 1.60.0 (as we say), because a dependency of multihash (in the latest version), arbitrary (also latest version), uses |
|
At the moment, we have a Note that we only need to run a The matrix for stable and beta should only apply to tests and clippy. Those we can run on the full workspace as we currently do. |
|
But how should i handle the invalid rust-version? |
We just need to set it to the correct one, i.e. what it effectively is. |
|
#2996 mentions another error case where a regular |
|
Not running CI on beta causes unrelated CI failures on new stable releases: https://github.com/libp2p/rust-libp2p/actions/runs/3390658695/jobs/5635053146 |
|
As noted on #2289, it would be useful to have a feature for all protocols/transports and then selectively test whether it all builds with only |
thomaseizinger
changed the title
We refactor our continuous integration workflow with the following goals in mind: - Run as few jobs as possible - Have the jobs finish as fast as possible - Have the jobs redo as little work as possible There are only so many jobs that GitHub Actions will run in parallel. Thus, it makes sense to not create massive matrices but instead group things together meaningfully. The new `test` job will: - Run once for each crate - Ensure that the crate compiles on its specified MSRV - Ensure that the tests pass - Ensure that there are no semver violations This is an improvement to before because we are running all of these in parallel which speeds up execution and highlights more errors at once. Previously, tests run later in the pipeline would not get run at all until you make sure the "first" one passes. We also previously did not verify the MSRV of each crate, making the setting in the `Cargo.toml` rather pointless. The new `cross` job supersedes the existing `wasm` job. This is an improvement because we now also compile the crate for windows and MacOS. Something that wasn't checked before. We assume that checking MSRV and the tests under Linux is good enough. Hence, this job only checks for compile-errors. The new `feature_matrix` ensures we compile correctly with certain feature combinations. `libp2p` exposes a fair few feature-flags. Some of the combinations are worth checking independently. For the moment, this concerns only the executor related transports together with the executor flags but this list can easily be extended. The new `clippy` job runs for `stable` and `beta` rust. Clippy gets continuously extended with new lints. Up until now, we would only learn about those as soon as a new version of Rust is released and CI would run the new lints. This leads to unrelated failures in CI. Running clippy on with `beta` Rust gives us a heads-up of 6 weeks before these lints land on stable. Fixes #2951.
We refactor our continuous integration workflow with the following goals in mind: - Run as few jobs as possible - Have the jobs finish as fast as possible - Have the jobs redo as little work as possible There are only so many jobs that GitHub Actions will run in parallel. Thus, it makes sense to not create massive matrices but instead group things together meaningfully. The new `test` job will: - Run once for each crate - Ensure that the crate compiles on its specified MSRV - Ensure that the tests pass - Ensure that there are no semver violations This is an improvement to before because we are running all of these in parallel which speeds up execution and highlights more errors at once. Previously, tests run later in the pipeline would not get run at all until you make sure the "first" one passes. We also previously did not verify the MSRV of each crate, making the setting in the `Cargo.toml` rather pointless. The new `cross` job supersedes the existing `wasm` job. This is an improvement because we now also compile the crate for windows and MacOS. Something that wasn't checked before. We assume that checking MSRV and the tests under Linux is good enough. Hence, this job only checks for compile-errors. The new `feature_matrix` ensures we compile correctly with certain feature combinations. `libp2p` exposes a fair few feature-flags. Some of the combinations are worth checking independently. For the moment, this concerns only the executor related transports together with the executor flags but this list can easily be extended. The new `clippy` job runs for `stable` and `beta` rust. Clippy gets continuously extended with new lints. Up until now, we would only learn about those as soon as a new version of Rust is released and CI would run the new lints. This leads to unrelated failures in CI. Running clippy on with `beta` Rust gives us a heads-up of 6 weeks before these lints land on stable. Fixes libp2p#2951.
Description
Change our CI to run on:
Motivation
MSRV: We set a rust-version in our manifest but we don't actually run CI against that version.
Beta: Rust beta's are cut 6 week before landing on stable. This gives us enough time to know ahead of time, which lints are coming and also to notify the Rust devs in case anything that is about to hit stable breaks our build.
Current Implementation
We only run CI against latest stable Rust.
Are you planning to do it yourself in a pull request?
Maybe.
The text was updated successfully, but these errors were encountered: