-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Make it possible to run cargo test
for bootstrap
#95253
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
src/bootstrap/builder/tests.rs
Outdated
config.out = PathBuf::from(env::var_os("BOOTSTRAP_OUTPUT_DIRECTORY").unwrap()); | ||
config.initial_rustc = PathBuf::from(env::var_os("RUSTC").unwrap()); | ||
config.initial_cargo = PathBuf::from(env::var_os("BOOTSTRAP_INITIAL_CARGO").unwrap()); | ||
config.out = PathBuf::from(env!("OUT_DIR")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh. I'm surprised this is the same behavior-wise:
If the package has a build script, this is set to the folder where the build script should place its output. See below for more information. (Only set during compilation.)
That doesn't really seem like what we want? Or at least, it seems a little weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, good point, cargo will probably cache these temporary files indefinitely. One alternative is to just use the out
determined by Config::parse
? But unfortunately cargo runs these tests with a working directory of src/bootstrap
, so it will use src/bootstrap/build
instead of the top-level build file :( you mentioned in another PR adding tempdir
seems overkill, so I think OUT_DIR
is probably an OK alternative? at least, it shouldn't hurt anything, nothing else in bootstrap reads from OUT_DIR.
I'm interested in hearing what the next steps for cargo test are. Are you intending to add support for anything other than bootstrap itself to get run via cargo test? (It seems possible though somewhat annoying). |
Ooh, this is an excellent idea :D I think it wouldn't be too terrible to do if we added an integration test in |
Note that this only runs bootstrap's self-tests, not compiler or library tests.
@rustbot ready |
@bors r+ |
📌 Commit a0de44f has been approved by |
⌛ Testing commit a0de44f with merge 693dcc53bd2ca16987fd5b93582444b0ff33534b... |
💔 Test failed - checks-actions |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@bors retry stdio-from I/O error: operation failed to complete synchronously msvc |
☀️ Test successful - checks-actions |
Finished benchmarking commit (32c2630): comparison url. Summary:
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression Footnotes |
Note that this only runs bootstrap's self-tests, not compiler or library tests.
Helps with #94829.