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

Parallelize and workaround CI style checks #1184

Merged
merged 4 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 36 additions & 21 deletions .github/scripts/ci-style.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,51 @@ if [[ $CLIPPY_VERSION == "clippy 0.1.72"* ]]; then
export CARGO_INCREMENTAL=0
fi

if [[ $CLIPPY_VERSION == "clippy 0.1.77"* && $CARGO_BUILD_TARGET == "x86_64-apple-darwin" ]]; then
export SKIP_CLIPPY=1
fi

# --- Check main crate ---

# check base
cargo clippy
# check all features
for_all_features "cargo clippy"
# check release
for_all_features "cargo clippy --release"
# check for tests
for_all_features "cargo clippy --tests"

# target-specific features
if [[ $arch == "x86_64" && $os == "linux" ]]; then
cargo clippy --features perf_counter
cargo clippy --release --features perf_counter
cargo clippy --tests --features perf_counter
fi
if [[ $SKIP_CLIPPY == 1 ]]; then
echo "Skipping clippy version $CLIPPY_VERSION on $CARGO_BUILD_TARGET"
else
# check base
cargo clippy
# check all features
for_all_features "cargo clippy"
# check release
for_all_features "cargo clippy --release"
# check for tests
for_all_features "cargo clippy --tests"

# mock tests
cargo clippy --features mock_test
cargo clippy --features mock_test --tests
cargo clippy --features mock_test --benches
# target-specific features
if [[ $arch == "x86_64" && $os == "linux" ]]; then
cargo clippy --features perf_counter
cargo clippy --release --features perf_counter
cargo clippy --tests --features perf_counter
fi

# mock tests
cargo clippy --features mock_test
cargo clippy --features mock_test --tests
cargo clippy --features mock_test --benches

# non-mock benchmarks
cargo clippy --features test_private --benches
fi

# --- Check auxiliary crate ---

style_check_auxiliary_crate() {
crate_path=$1

cargo clippy --manifest-path=$crate_path/Cargo.toml
cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
if [[ $SKIP_CLIPPY == 1 ]]; then
echo "Skipping clippy test for $crate_path"
else
cargo clippy --manifest-path=$crate_path/Cargo.toml
cargo fmt --manifest-path=$crate_path/Cargo.toml -- --check
fi
}

style_check_auxiliary_crate macros
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/merge-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ env:
# Ignore some actions for the merge check:
# - This action itself
# - Public API check, doc broken link check: we allow them to fail.
# - Minimal tests for stable Rust: we allow them to fail.
# - Minimal tests and style checks for stable Rust: we allow them to fail.
# - Extended binding tests: it may take long to run. We don't want to wait for them.
# Note: The action name for openjdk tests is different due to the reused workflow.
IGNORED_ACTIONS: |
Expand All @@ -23,6 +23,9 @@ env:
"minimal-tests-core/x86_64-unknown-linux-gnu/stable",
"minimal-tests-core/i686-unknown-linux-gnu/stable",
"minimal-tests-core/x86_64-apple-darwin/stable",
"style-check/x86_64-unknown-linux-gnu/stable",
"style-check/i686-unknown-linux-gnu/stable",
"style-check/x86_64-apple-darwin/stable",
"extended-tests-openjdk / test",
"extended-tests-v8",
"extended-tests-jikesrvm",
Expand Down
43 changes: 43 additions & 0 deletions .github/workflows/minimal-tests-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,49 @@ jobs:
- name: Test
run: ./.github/scripts/ci-test.sh

style-check:
needs: setup-test-matrix
strategy:
fail-fast: false
matrix:
target:
- { os: ubuntu-22.04, triple: x86_64-unknown-linux-gnu }
- { os: ubuntu-22.04, triple: i686-unknown-linux-gnu }
- { os: macos-12, triple: x86_64-apple-darwin }
rust: ${{ fromJson(needs.setup-test-matrix.outputs.rust )}}

name: style-check/${{ matrix.target.triple }}/${{ matrix.rust }}
runs-on: ${{ matrix.target.os }}

env:
# This determines the default target which cargo-build, cargo-test, etc. use.
CARGO_BUILD_TARGET: "${{ matrix.target.triple }}"

steps:
- uses: actions/checkout@v4
- name: Install Rust
run: |
# "rustup toolchain install" should always install the host toolchain,
# so we don't specify the triple.
rustup toolchain install ${{ matrix.rust }}
rustup override set ${{ matrix.rust }}

# Ensure we install the target support for the target we are testing for.
# This is especially important for i686-unknown-linux-gnu
# because it's different from the host.
rustup target add ${{ matrix.target.triple }}

rustup component add rustfmt clippy

# Show the Rust toolchain and target we are actually using
- run: rustup show
- run: cargo --version
- run: cargo rustc -- --print cfg

# Setup Environments
- name: Setup Environments
run: ./.github/scripts/ci-setup-${{ matrix.target.triple }}.sh

# Style checks
- name: Style checks
run: ./.github/scripts/ci-style.sh
Expand Down
6 changes: 3 additions & 3 deletions benches/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ pub mod mock_bench;
#[cfg(all(not(feature = "mock_test"), feature = "test_private"))]
pub mod regular_bench;

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