Skip to content

Commit 38b3fc3

Browse files
authored
Parallelize and workaround CI style checks (#1184)
We now do style checks in parallel to `cargo test` so that style problems will fail fast. Black-listed clippy 0.1.77 on x86_64-apple-darwin because it is known to crash randomly. Also added missing style checks for non-mock benchmarks.
1 parent 8623b77 commit 38b3fc3

File tree

4 files changed

+86
-25
lines changed

4 files changed

+86
-25
lines changed

Diff for: .github/scripts/ci-style.sh

+36-21
Original file line numberDiff line numberDiff line change
@@ -13,36 +13,51 @@ if [[ $CLIPPY_VERSION == "clippy 0.1.72"* ]]; then
1313
export CARGO_INCREMENTAL=0
1414
fi
1515

16+
if [[ $CLIPPY_VERSION == "clippy 0.1.77"* && $CARGO_BUILD_TARGET == "x86_64-apple-darwin" ]]; then
17+
export SKIP_CLIPPY=1
18+
fi
19+
1620
# --- Check main crate ---
1721

18-
# check base
19-
cargo clippy
20-
# check all features
21-
for_all_features "cargo clippy"
22-
# check release
23-
for_all_features "cargo clippy --release"
24-
# check for tests
25-
for_all_features "cargo clippy --tests"
26-
27-
# target-specific features
28-
if [[ $arch == "x86_64" && $os == "linux" ]]; then
29-
cargo clippy --features perf_counter
30-
cargo clippy --release --features perf_counter
31-
cargo clippy --tests --features perf_counter
32-
fi
22+
if [[ $SKIP_CLIPPY == 1 ]]; then
23+
echo "Skipping clippy version $CLIPPY_VERSION on $CARGO_BUILD_TARGET"
24+
else
25+
# check base
26+
cargo clippy
27+
# check all features
28+
for_all_features "cargo clippy"
29+
# check release
30+
for_all_features "cargo clippy --release"
31+
# check for tests
32+
for_all_features "cargo clippy --tests"
3333

34-
# mock tests
35-
cargo clippy --features mock_test
36-
cargo clippy --features mock_test --tests
37-
cargo clippy --features mock_test --benches
34+
# target-specific features
35+
if [[ $arch == "x86_64" && $os == "linux" ]]; then
36+
cargo clippy --features perf_counter
37+
cargo clippy --release --features perf_counter
38+
cargo clippy --tests --features perf_counter
39+
fi
40+
41+
# mock tests
42+
cargo clippy --features mock_test
43+
cargo clippy --features mock_test --tests
44+
cargo clippy --features mock_test --benches
45+
46+
# non-mock benchmarks
47+
cargo clippy --features test_private --benches
48+
fi
3849

3950
# --- Check auxiliary crate ---
4051

4152
style_check_auxiliary_crate() {
4253
crate_path=$1
4354

44-
cargo clippy --manifest-path=$crate_path/Cargo.toml
45-
cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
55+
if [[ $SKIP_CLIPPY == 1 ]]; then
56+
echo "Skipping clippy test for $crate_path"
57+
else
58+
cargo clippy --manifest-path=$crate_path/Cargo.toml
59+
cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
60+
fi
4661
}
4762

4863
style_check_auxiliary_crate macros

Diff for: .github/workflows/merge-check.yml

+4-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ env:
1111
# Ignore some actions for the merge check:
1212
# - This action itself
1313
# - Public API check, doc broken link check: we allow them to fail.
14-
# - Minimal tests for stable Rust: we allow them to fail.
14+
# - Minimal tests and style checks for stable Rust: we allow them to fail.
1515
# - Extended binding tests: it may take long to run. We don't want to wait for them.
1616
# Note: The action name for openjdk tests is different due to the reused workflow.
1717
IGNORED_ACTIONS: |
@@ -23,6 +23,9 @@ env:
2323
"minimal-tests-core/x86_64-unknown-linux-gnu/stable",
2424
"minimal-tests-core/i686-unknown-linux-gnu/stable",
2525
"minimal-tests-core/x86_64-apple-darwin/stable",
26+
"style-check/x86_64-unknown-linux-gnu/stable",
27+
"style-check/i686-unknown-linux-gnu/stable",
28+
"style-check/x86_64-apple-darwin/stable",
2629
"extended-tests-openjdk / test",
2730
"extended-tests-v8",
2831
"extended-tests-jikesrvm",

Diff for: .github/workflows/minimal-tests-core.yml

+43
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,49 @@ jobs:
7979
- name: Test
8080
run: ./.github/scripts/ci-test.sh
8181

82+
style-check:
83+
needs: setup-test-matrix
84+
strategy:
85+
fail-fast: false
86+
matrix:
87+
target:
88+
- { os: ubuntu-22.04, triple: x86_64-unknown-linux-gnu }
89+
- { os: ubuntu-22.04, triple: i686-unknown-linux-gnu }
90+
- { os: macos-12, triple: x86_64-apple-darwin }
91+
rust: ${{ fromJson(needs.setup-test-matrix.outputs.rust )}}
92+
93+
name: style-check/${{ matrix.target.triple }}/${{ matrix.rust }}
94+
runs-on: ${{ matrix.target.os }}
95+
96+
env:
97+
# This determines the default target which cargo-build, cargo-test, etc. use.
98+
CARGO_BUILD_TARGET: "${{ matrix.target.triple }}"
99+
100+
steps:
101+
- uses: actions/checkout@v4
102+
- name: Install Rust
103+
run: |
104+
# "rustup toolchain install" should always install the host toolchain,
105+
# so we don't specify the triple.
106+
rustup toolchain install ${{ matrix.rust }}
107+
rustup override set ${{ matrix.rust }}
108+
109+
# Ensure we install the target support for the target we are testing for.
110+
# This is especially important for i686-unknown-linux-gnu
111+
# because it's different from the host.
112+
rustup target add ${{ matrix.target.triple }}
113+
114+
rustup component add rustfmt clippy
115+
116+
# Show the Rust toolchain and target we are actually using
117+
- run: rustup show
118+
- run: cargo --version
119+
- run: cargo rustc -- --print cfg
120+
121+
# Setup Environments
122+
- name: Setup Environments
123+
run: ./.github/scripts/ci-setup-${{ matrix.target.triple }}.sh
124+
82125
# Style checks
83126
- name: Style checks
84127
run: ./.github/scripts/ci-style.sh

Diff for: benches/main.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ pub mod mock_bench;
88
#[cfg(all(not(feature = "mock_test"), feature = "test_private"))]
99
pub mod regular_bench;
1010

11-
pub fn bench_main(c: &mut Criterion) {
11+
pub fn bench_main(_c: &mut Criterion) {
1212
cfg_if::cfg_if! {
1313
if #[cfg(feature = "mock_test")] {
1414
// If the "mock_test" feature is enabled, we only run mock test.
15-
mock_bench::bench(c);
15+
mock_bench::bench(_c);
1616
} else if #[cfg(feature = "test_private")] {
17-
regular_bench::bench(c);
17+
regular_bench::bench(_c);
1818
} else {
1919
eprintln!("ERROR: Benchmarks in mmtk_core requires the test_priavte feature (implied by mock_test) to run.");
2020
eprintln!(" Rerun with `MMTK_BENCH=\"bench_name\" cargo bench --features mock_test` to run mock-test benchmarks.");

0 commit comments

Comments
 (0)