Skip to content
/ rust Public
forked from rust-lang/rust

Commit ecaa35f

Browse files
authored
Rollup merge of rust-lang#139015 - Kobzol:llvm-ci-test-fixes, r=onur-ozkan
Remove unneeded LLVM CI test assertions The `download_ci_llvm` bootstrap test was checking implementation details of the LLVM CI download check, which isn't very useful. It was essentially testing "if function_that_checks_if_llvm_ci_is_available returns true, we enable CI LLVM", but the usage of the function was an implementation detail. After rust-lang#138704, the inner implementation has changed, so the test now breaks if LLVM is updated. I don't think that it's very useful to test implementation details like this, without taking the outside git state into account. Ideally, we should mock the git state for the test, otherwise the test will randomly break when executed in environments which the test does not control (e.g. on CI when a LLVM change happens). I only kept the part of the test that checks that LLVM CI isn't used when we specify `download-ci-llvm = false`, as that should hold under all conditions, CI/local, and all git states. I also kept the `if-unchanged` assertion, but only on CI, and as a temporary measure. After rust-lang#138591, we should have a proper way of mocking the git state to make the test robust, and make it test what we actually want. Fixes [this](rust-lang#138784 (comment)). r? `@ghost`
2 parents d517a4f + 215c2c2 commit ecaa35f

File tree

1 file changed

+1
-21
lines changed

1 file changed

+1
-21
lines changed

Diff for: src/bootstrap/src/core/config/tests.rs

+1-21
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,11 @@ pub(crate) fn parse(config: &str) -> Config {
2424

2525
#[test]
2626
fn download_ci_llvm() {
27-
let config = parse("");
28-
let is_available = llvm::is_ci_llvm_available_for_target(&config, config.llvm_assertions);
29-
if is_available {
30-
assert!(config.llvm_from_ci);
31-
}
32-
33-
let config = Config::parse_inner(
34-
Flags::parse(&[
35-
"check".to_string(),
36-
"--config=/does/not/exist".to_string(),
37-
"--ci".to_string(),
38-
"false".to_string(),
39-
]),
40-
|&_| toml::from_str("llvm.download-ci-llvm = true"),
41-
);
42-
let is_available = llvm::is_ci_llvm_available_for_target(&config, config.llvm_assertions);
43-
if is_available {
44-
assert!(config.llvm_from_ci);
45-
}
46-
4727
let config = parse("llvm.download-ci-llvm = false");
4828
assert!(!config.llvm_from_ci);
4929

5030
let if_unchanged_config = parse("llvm.download-ci-llvm = \"if-unchanged\"");
51-
if if_unchanged_config.llvm_from_ci {
31+
if if_unchanged_config.llvm_from_ci && if_unchanged_config.is_running_on_ci {
5232
let has_changes = if_unchanged_config
5333
.last_modified_commit(LLVM_INVALIDATION_PATHS, "download-ci-llvm", true)
5434
.is_none();

0 commit comments

Comments
 (0)