Skip to content

difftest: nextest support and speedups #334

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[alias]
compiletest = "run --release -p compiletests --"
difftest = "run --release -p difftests --"
difftest = "nextest run --release -P difftests -p difftests"


[target.x86_64-pc-windows-msvc]
Expand Down
10 changes: 10 additions & 0 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[profile.default]
default-filter = '!package(difftest*) & !package(compiletest*) & !package(example-runner-*)'
fail-fast = false

[profile.difftests]
default-filter = 'package(difftests)'
slow-timeout = "2m"

[profile.difftest-runner]
default-filter = 'package(difftest-runner) | package(difftest-types)'
21 changes: 15 additions & 6 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ jobs:
test:
name: Test
strategy:
fail-fast: false
matrix:
os: [ ubuntu-24.04, windows-2022, macOS-latest ]
runs-on: ${{ matrix.os }}
Expand All @@ -41,6 +42,8 @@ jobs:
# figure out native target triple while we're at it
- name: install rust-toolchain
run: echo "TARGET=$(rustc --print host-tuple)" >> "$GITHUB_ENV"
- name: install nextest
uses: taiki-e/install-action@nextest
# Fetch dependencies in a separate step to clearly show how long each part
# of the testing takes
- name: cargo fetch --locked
Expand All @@ -49,13 +52,13 @@ jobs:
# Core crates
# Compiled in --release because cargo compiletest would otherwise compile in release again.
- name: rustc_codegen_spirv build
run: cargo test -p rustc_codegen_spirv --release --no-default-features --features "use-installed-tools" --no-run
run: cargo nextest run -p rustc_codegen_spirv --release --no-default-features --features "use-installed-tools" --no-run

- name: rustc_codegen_spirv test
run: cargo test -p rustc_codegen_spirv --release --no-default-features --features "use-installed-tools"
run: cargo nextest run -p rustc_codegen_spirv --release --no-default-features --features "use-installed-tools"

- name: workspace test (excluding examples & difftest)
run: cargo test --release --workspace --exclude "example-runner-*" --exclude "difftest*" --no-default-features --features "use-installed-tools"
run: cargo nextest run --release --workspace --exclude "example-runner-*" --exclude "difftest*" --no-default-features --features "use-installed-tools"

# Examples
- name: cargo check examples
Expand Down Expand Up @@ -129,6 +132,7 @@ jobs:
compiletest:
name: Compiletest
strategy:
fail-fast: false
matrix:
os: [ ubuntu-24.04, windows-2022, macOS-latest ]
runs-on: ${{ matrix.os }}
Expand All @@ -154,6 +158,7 @@ jobs:
difftest:
name: Difftest
strategy:
fail-fast: false
matrix:
os: [ ubuntu-24.04, windows-2022, macOS-latest ]
runs-on: ${{ matrix.os }}
Expand Down Expand Up @@ -181,14 +186,18 @@ jobs:
sudo apt install -y xvfb libgl1-mesa-dri libxcb-xfixes0-dev mesa-vulkan-drivers
- name: install rust-toolchain
run: echo "TARGET=$(rustc --print host-tuple)" >> "$GITHUB_ENV"
- name: install nextest
uses: taiki-e/install-action@nextest
- name: cargo fetch --locked
run: cargo fetch --locked --target $TARGET
- name: cargo fetch --locked difftests
run: cargo fetch --locked --manifest-path=tests/difftests/tests/Cargo.toml --target $TARGET
- name: test difftest
run: cargo test -p "difftest*" --release --no-default-features --features "use-installed-tools"
- name: test difftest-runner
Copy link
Collaborator

Choose a reason for hiding this comment

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

My main concern here is we are making CI smarter when best practice is to make it as dumb as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still a little unsure what to do about precompiling the tests, which is not just an CI issue but also when building from your own machine. Currently, it'll look like the tests are just stuck eating CPU for minutes, apart from the occasional slow warning from nextest, as all tests are blocked on the first test holding the target folder lock and compiling all dependencies. So at least for CI, instead of windows taking 7-8min of "nothing happening", we get cargo's compilation progress reporting.

We could move precompiling tests to the listing stage, but that would make nextest list take forever on first execution. Not even sure if we can even print compilation progress, as when listing tests we must only write a list of tests to stdout and nothing else, otherwise nextest will error out. Though I haven't tested if piping through stderr would work.

Maybe we could alias cargo difftest to be proceeded by a test compile before running nextest?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't that basically what we had? It ran cargo build --release in the tests workspace regardless of what was running. The thinking was that most of the deps would be shared, but I guess I soon as I added ash vs wgpu that went out the window.

Copy link
Member Author

Choose a reason for hiding this comment

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

Crates in a workspace will compile all their dependencies into the target dir of their workspace, regardless of where they're from. So on master, the difftest lib crate would be compiled once in the difftest workspace's target and a second time in the root workspace's target. Now only types is shared like that, and a lot lighter since it's just plain types and no wgpu or ash.

run: cargo nextest run -P difftest-runner -p difftest-runner -p difftest-types --release --no-default-features --features "use-installed-tools"
- name: build difftests (without shaders)
run: cargo build --manifest-path ./tests/difftests/tests/Cargo.toml --workspace --release --no-default-features --features "use-installed-tools"
- name: difftests
run: cargo run -p difftests --release --no-default-features --features "use-installed-tools"
run: cargo nextest run -P difftests -p difftests --release --no-default-features --features "use-installed-tools"

# This allows us to have a single job we can branch protect on, rather than needing
# to update the branch protection rules when the test matrix changes
Expand Down
75 changes: 36 additions & 39 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 10 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ members = [
"tests/compiletests",
"tests/compiletests/deps-helper",
"tests/difftests/bin",
"tests/difftests/lib",
"tests/difftests/runner",
"tests/difftests/types",
]

[workspace.package]
Expand All @@ -50,14 +51,20 @@ spirv-tools = { version = "0.12.1", default-features = false }
rustc_codegen_spirv = { path = "./crates/rustc_codegen_spirv", version = "=0.9.0", default-features = false }
rustc_codegen_spirv-types = { path = "./crates/rustc_codegen_spirv-types", version = "=0.9.0" }
rustc_codegen_spirv-target-specs = { path = "crates/rustc_codegen_spirv-target-specs", version = "=0.9.0" }
tracing = "0.1"
tracing-subscriber = { version = "0.3.3", features = ["env-filter", "json"] }

# difftest libraries mirrored from difftest workspace
difftest = { path = "tests/difftests/tests/lib" }
difftest-runner = { path = "tests/difftests/runner", default-features = false }
difftest-types = { path = "tests/difftests/types" }

# External dependencies that need to be mentioned more than once.
tracing = "0.1"
tracing-subscriber = { version = "0.3.3", features = ["env-filter", "json"] }
num-traits = { version = "0.2.15", default-features = false }
glam = { version = ">=0.22, <=0.30", default-features = false }
# libm 0.2.12 is a breaking change with new intrinsics
libm = { version = ">=0.2.5, <=0.2.11", default-features = false }
bytemuck = { version = "1.23", features = ["derive"] }

# Enable incremental by default in release mode.
[profile.release]
Expand Down
2 changes: 1 addition & 1 deletion crates/rustc_codegen_spirv/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ skip-toolchain-check = []
# HACK(eddyb) these only exist to unify features across dependency trees,
# in order to avoid multiple separate instances of `rustc_codegen_spirv`.
ahash = { version = "0.8.11", features = ["no-rng"] }
bytemuck = { version = "1.20.0", features = ["aarch64_simd", "derive"] }
bytemuck = { workspace = true, features = ["aarch64_simd"] }
log = { version = "0.4.22", features = ["std"] }
regex = { version = "1", features = ["perf"] }
rustix = { version = "0.38.42", features = ["all-apis"] }
Expand Down
2 changes: 1 addition & 1 deletion crates/spirv-std/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ workspace = true
spirv-std-types.workspace = true
spirv-std-macros.workspace = true
bitflags = "1.2.1"
bytemuck = { version = "1.18.0", features = ["derive"], optional = true }
bytemuck = { workspace = true, optional = true }

[target.'cfg(target_arch = "spirv")'.dependencies]
num-traits = { workspace = true, features = ["libm"] }
Expand Down
22 changes: 8 additions & 14 deletions tests/difftests/bin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,17 @@ repository.workspace = true
# See rustc_codegen_spirv/Cargo.toml for details on these features
[features]
default = ["use-compiled-tools"]
use-installed-tools = []
use-compiled-tools = []
use-installed-tools = ["difftest-runner/use-installed-tools"]
use-compiled-tools = ["difftest-runner/use-compiled-tools"]

[dependencies]
anyhow = "1.0"
tracing = "0.1"
tracing-subscriber = { version = "0.3", features = ["fmt", "env-filter"] }
tempfile = "3.5"
tester = "0.9.1"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
thiserror = "1.0"
toml = { version = "0.8.20", default-features = false, features = ["parse"] }
bytesize = "2.0.1"
bytemuck = "1.21.0"
difftest = { path = "../lib" }
tabled = { version = "0.15", default-features = false, features = ["std"] }
difftest-runner.workspace = true

[lints]
workspace = true

[[test]]
name = "difftests"
path = "src/test.rs"
harness = false
Loading
Loading