Skip to content

Commit

Permalink
Remove various test file generation features
Browse files Browse the repository at this point in the history
Remove various test file generation features from the main crate. These
features will from now on only be present on blazesym-dev. The reason
for the removal is that the existing main-crate features are broken.
Evidently, Cargo does not correctly handle dependent features if
triggered via a dev-dependency. As a result, it turns out that our
Windows tests were broken, because the build-test-artifacts job, by
virtue of relying on the generate-unit-test-files on the main crate, was
not generating and uploading the expected set of test artifacts.

Signed-off-by: Daniel Müller <[email protected]>
  • Loading branch information
d-e-s-o committed Oct 16, 2024
1 parent f1e6435 commit 32bd801
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 29 deletions.
23 changes: 12 additions & 11 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ jobs:
runs-on: [ubuntu-latest, macos-latest]
rust: [stable]
profile: [dev, release]
# gsym-in-apk requires `generate-unit-test-files` feature, which
# requires `llvm-gsymutil` installed. We don't want that dependency in
# this large OS spanning matrix.
# gsym-in-apk requires `blazesym-dev/generate-unit-test-files`
# feature, which requires `llvm-gsymutil` installed. We don't
# want that dependency in this large OS spanning matrix.
args: ["--workspace --exclude=gsym-in-apk"]
include:
- runs-on: ubuntu-latest
Expand All @@ -52,7 +52,7 @@ jobs:
- runs-on: windows-latest
rust: stable
profile: dev
args: "--tests --features=dont-generate-unit-test-files"
args: "--tests --features=blazesym-dev/dont-generate-unit-test-files"
- runs-on: ubuntu-latest
rust: stable
profile: dev
Expand Down Expand Up @@ -137,7 +137,7 @@ jobs:
run: sudo apt-get install -y llvm-14
- name: Check incremental rebuilds
env:
CARGO_OPTS: --features=generate-unit-test-files --workspace --quiet --tests
CARGO_OPTS: --workspace --quiet --tests
run: |
cargo check ${CARGO_OPTS}
# We need another build here to have the reference `output` file
Expand Down Expand Up @@ -175,6 +175,7 @@ jobs:
env:
ARTIFACT_URL: ${{ needs.build-linux-kernel.outputs.kernel-artifact-url }}
PYTHON: ${{ steps.py312.outputs.python-path }}
RUSTFLAGS: '--cfg has_large_test_files'
run: |
# cargo-llvm-cov ultimately is just a thin wrapper around cargo
# commands with certain environment variables set. Because it
Expand All @@ -185,7 +186,7 @@ jobs:
echo "export PYTHON=${PYTHON}" >> env.sh
source env.sh
cargo test --no-run --workspace --all-targets --features=nightly,generate-large-test-files 2>&1 | tee /tmp/cargo-build.log
cargo test --no-run --workspace --all-targets --features=nightly,blazesym-dev/generate-large-test-files 2>&1 | tee /tmp/cargo-build.log
tests=$(IFS='' grep Executable /tmp/cargo-build.log |
awk '{print $NF}' |
sed 's@[()]@@g' |
Expand Down Expand Up @@ -241,7 +242,7 @@ jobs:
run: sudo apt-get install -y llvm-14
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@v2
- run: cargo build --features=generate-unit-test-files
- run: cargo check --package=blazesym-dev --features=generate-unit-test-files
- uses: actions/upload-artifact@v4
with:
name: test-stable-addrs
Expand All @@ -261,7 +262,7 @@ jobs:
path: data/
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@v2
- run: cargo test --tests --release --features=dont-generate-unit-test-files -- ':windows:'
- run: cargo test --tests --release --features=blazesym-dev/dont-generate-unit-test-files -- ':windows:'
test-sanitizers:
name: Test with ${{ matrix.sanitizer }} sanitizer
strategy:
Expand Down Expand Up @@ -327,7 +328,7 @@ jobs:
- name: Remove .cargo/config.toml
run: rm .cargo/config.toml
- name: Run tests
run: cargo miri test --workspace --features=dont-generate-unit-test-files -- ":miri:"
run: cargo miri test --workspace --features=blazesym-dev/dont-generate-unit-test-files -- ":miri:"
test-examples:
name: Test examples
runs-on: ubuntu-22.04
Expand Down Expand Up @@ -376,15 +377,15 @@ jobs:
# `--output-format` option, we need to specify the benchmark
# binary (`main`) here and have a different invocation for
# libtest style benchmarks above. Sigh.
cargo bench --workspace --bench=main --bench=capi --features=generate-large-test-files,dont-generate-unit-test-files -- --output-format=bencher | tee --append $GITHUB_STEP_SUMMARY
cargo bench --workspace --bench=main --bench=capi --features=blazesym-dev/generate-large-test-files,blazesym-dev/dont-generate-unit-test-files -- --output-format=bencher | tee --append $GITHUB_STEP_SUMMARY
echo '```' >> $GITHUB_STEP_SUMMARY
clippy:
name: Lint with clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: cargo clippy --workspace --no-deps --all-targets --features=dont-generate-unit-test-files -- -A unknown_lints -D clippy::todo
- run: cargo clippy --workspace --no-deps --all-targets --features=blazesym-dev/dont-generate-unit-test-files -- -A unknown_lints -D clippy::todo
rustfmt:
name: Check code formatting
runs-on: ubuntu-latest
Expand Down
15 changes: 3 additions & 12 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,6 @@ zstd = ["dep:zstd"]
# Below here are dev-mostly features that should not be needed by
# regular users.

# Enable this feature to opt in to the generation of unit test files.
# Having these test files created is necessary for running tests.
generate-unit-test-files = ["blazesym-dev/generate-unit-test-files"]
# Enable this feature to opt in to the generation of large benchmark
# files (also used for regression testing).
generate-large-test-files = ["blazesym-dev/generate-large-test-files"]
# Disable generation of test files. This feature takes preference over
# `generate-unit-test-files`.
dont-generate-unit-test-files = ["blazesym-dev/dont-generate-unit-test-files"]
# Enable code paths requiring a nightly toolchain. This feature is only meant to
# be used for testing and benchmarking purposes, not for the core library, which
# is expected to work on stable.
Expand All @@ -101,7 +92,7 @@ name = "normalize-virt-offset"

[[example]]
name = "inspect-mangled"
required-features = ["demangle", "generate-unit-test-files"]
required-features = ["demangle", "blazesym-dev/generate-unit-test-files"]

[[bench]]
name = "main"
Expand Down Expand Up @@ -138,7 +129,7 @@ anyhow = "1.0.71"
# TODO: Enable `zstd` feature once toolchain support for it is more
# widespread (enabled by default in `ld`). Remove conditionals in
# test code alongside.
blazesym = {path = ".", features = ["generate-unit-test-files", "apk", "breakpad", "gsym", "tracing"]}
blazesym = {path = ".", features = ["apk", "breakpad", "gsym", "tracing"]}
blazesym-dev = {path = "dev", features = ["generate-unit-test-files"]}
# TODO: Use 0.5.2 once released.
criterion = {git = "https://github.com/bheisler/criterion.rs.git", rev = "b913e232edd98780961ecfbae836ec77ede49259", default-features = false, features = ["rayon", "cargo_bench_support"]}
Expand All @@ -162,4 +153,4 @@ features = ["apk", "backtrace", "breakpad", "demangle", "dwarf", "gsym"]
rustdoc-args = ["--cfg", "docsrs"]

[lints.rust]
unexpected_cfgs = {level = "warn", check-cfg = ['cfg(has_procmap_query_ioctl)']}
unexpected_cfgs = {level = "warn", check-cfg = ['cfg(has_procmap_query_ioctl)', 'cfg(has_large_test_files)']}
9 changes: 5 additions & 4 deletions README-devel.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ $ cargo test --workspace
```
runs the vast majority of tests. Tests require `sudo` to be set up properly, as
some of the functionality we rely on is privileged. Test artifacts are
transparently created as long as the `generate-unit-test-files` feature is
active, which is enabled by default for testing.
transparently created as long as the `generate-unit-test-files` feature for the
`blazesym-dev` package is active, which is enabled by default for
testing.

### Running Miri
[Miri][miri] is used for testing the crate for any undefined behavior.
Expand All @@ -47,8 +48,8 @@ based][libtest] unit-test style ones.
To run the benchmark suite, use:
```sh
# Perform one-time setup of required data.
$ cargo check --features=generate-large-test-files
$ cargo bench --features=nightly
$ cargo check --package blazesym-dev --features=generate-large-test-files
$ RUSTFLAGS='--cfg has_large_test_files' cargo bench --features=nightly
```

Some benchmarks require the `PROCMAP_QUERY` ioctl kernel functionality,
Expand Down
2 changes: 1 addition & 1 deletion examples/gsym-in-apk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ path = "main.rs"
blazesym = {version = "=0.2.0-rc.1", path = "../..", default-features = false, features = [
"apk",
"gsym",
"generate-unit-test-files",
]}
blazesym-dev = {path = "../../dev", features = ["generate-unit-test-files"]}
goblin = {version = "0.8.0", default-features = false, features = ["elf32", "elf64", "std"]}
zip = {version = "2.0.0", default-features = false}
2 changes: 1 addition & 1 deletion tests/blazesym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ fn symbolize_breakpad_inlined() {
/// ```
/// In the past we were unable to handle this case properly.
#[test]
#[cfg_attr(not(feature = "generate-large-test-files"), ignore)]
#[cfg_attr(not(has_large_test_files), ignore)]
fn symbolize_dwarf_complex() {
let test_dwarf = Path::new(&env!("CARGO_MANIFEST_DIR"))
.join("data")
Expand Down

0 comments on commit 32bd801

Please sign in to comment.