Repository navigation
Mock environment testing for bootstrap #102563
Copy link
Copy link
Open
Labels
A-technical-debtArea: Internal cleanup workArea: Internal cleanup workA-testsuiteArea: The testsuite used to check the correctness of rustcArea: The testsuite used to check the correctness of rustcC-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Description
There are lots of config knobs in bootstrap, and it is hard to understand how they interact. Consider two recent examples off the top of my head:
- Make the
cfeature forcompiler-builtinsan explicit opt-in #101833 (comment)- Broke the flag
is_rust_llvm
- Broke the flag
- bootstrap: Add llvm-has-rust-patches target option #101072 (comment)
- Implemented the aforementioned flag
- (Nearly) broke the flag
llvm_from_ci - Caught in review by someone who knew the implementation details of that flag
We should really catch these issues in tests, not review, and avoid relying too much on specific reviewers who know parts of the code.
Since bootstrap does I/O and interacts with external tooling, it is inherently difficult to test. The strategy I know involves mocking out the build environment (enough to make tests run in <~1s). From there you can record what the implementation does and take two approaches for the test:
- Assert on specific conditions, like "the flag
-D FOOwas passed to LLVM cmake" or "foo/bar/baz was moved to foo/bar/quux". - Record everything in a golden file for some "representative" set of configurations. You can update these with
--bless. This helps guard against unexpected behavior changes.
I think both are helpful. In fact, combining the two is exactly what we do in ui tests.
Reacted by jyn and Trevor Gross
Metadata
Metadata
Assignees
Labels
A-technical-debtArea: Internal cleanup workArea: Internal cleanup workA-testsuiteArea: The testsuite used to check the correctness of rustcArea: The testsuite used to check the correctness of rustcC-enhancementCategory: An issue proposing an enhancement or a PR with one.Category: An issue proposing an enhancement or a PR with one.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Activity
tmandry commentedon Oct 2, 2022
Implementation notes: Mocking can be implemented with traits (hopefully trait objects to avoid having so many generics). But for any "reading" from the environment you must specify some mock data that can be used in tests. For example, when reading from a file you must provide mock file contents that are used in tests.
Tip: Virtualizing the filesystem itself is not actually necessary. If it's easier, for example, we could run in a sandbox build directory set up for that test. For each external command we'd specify the expected output files with mock outputs and actually create them on the filesystem, then allow directly manipulating them (move/copy/read/etc.) within
bootstrapin the test sandbox. I could see this plugging in nicely to the existing traits we have.Mark-Simulacrum commentedon Oct 7, 2022
We already try to test some of this via
--dry-runwhich intends to run really quickly (it actually runs on every invocation before starting the real build, and we verify that the Steps executed are the same between both IIRC).I'm not sure how much extra is needed beyond that -- certainly it supports configuration. I think in order to test things like passing specific flags, we'd want to be dumping/preserving more state than we do today, but that doesn't seem particularly hard (just needs some plumbing to keep vectors of commands around or something).
jyn514 commentedon Feb 2, 2023
I don't think it's that simple - if nothing else,
outputdoesn't supportdry_runtoday, so anything usingoutputis special-casingdry_runand we can't see any command it would normally try to run. and in general I think there are lots of blocks that are wholly skipped when dry_run is enabled because it was simpler.jyn514 commentedon Feb 2, 2023
🙃
jyn514 commentedon Mar 17, 2023
@oli-obk asked me recently "why is this hard?" and I want to record those answers somewhere. A non-exhaustive things I'd like to test:
srcandout: add bootstrap tests for detecting src and out #109120outsrcis read-only?"originoriginsrc/tools/xwrapperonur-ozkan commentedon Apr 16, 2023
Also same for multiarch linux distros like Debian and Ubuntu
jyn514 commentedon Apr 16, 2023
oh yeah I'd love to test "do external tools using rustc_private reliably find the sysroot without hacks"; and all of these things but with
download-rustcenabled.Rollup merge of rust-lang#111607 - jyn514:clubby-reviews, r=clubby789
Rollup merge of rust-lang#111607 - jyn514:clubby-reviews, r=clubby789
Rollup merge of rust-lang#111607 - jyn514:clubby-reviews, r=clubby789
Rollup merge of rust-lang#111607 - jyn514:clubby-reviews, r=clubby789
Rollup merge of rust-lang#111607 - jyn514:clubby-reviews, r=clubby789
Rollup merge of #111607 - jyn514:clubby-reviews, r=clubby789