From 73e534b0220604cf79e2193d4f5960d2dda0a3b6 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Fri, 25 Jul 2025 19:47:54 +0800 Subject: Revert "Move `shared_helpers` test to a dedicated module" I missed this during review. We cannot declare a `tests` module within `shared_helpers.rs` itself, as shim binaries that tries to include the `shared_helpers` module through `#[path = ".."]` attributes would fail to find it, breaking `./x check bootstrap`. This reverts commit `40c2ca96411caaeab1563ff9041879f742d1d71b`. --- src/bootstrap/src/utils/shared_helpers.rs | 22 +++++++------- src/bootstrap/src/utils/shared_helpers/tests.rs | 28 ----------------- src/bootstrap/src/utils/tests/mod.rs | 4 +++ .../src/utils/tests/shared_helpers_tests.rs | 35 ++++++++++++++++++++++ 4 files changed, 50 insertions(+), 39 deletions(-) delete mode 100644 src/bootstrap/src/utils/shared_helpers/tests.rs create mode 100644 src/bootstrap/src/utils/tests/shared_helpers_tests.rs (limited to 'src') diff --git a/src/bootstrap/src/utils/shared_helpers.rs b/src/bootstrap/src/utils/shared_helpers.rs index 9c6b4a7615d..9428e221f41 100644 --- a/src/bootstrap/src/utils/shared_helpers.rs +++ b/src/bootstrap/src/utils/shared_helpers.rs @@ -1,14 +1,18 @@ //! This module serves two purposes: -//! 1. It is part of the `utils` module and used in other parts of bootstrap. -//! 2. It is embedded inside bootstrap shims to avoid a dependency on the bootstrap library. -//! Therefore, this module should never use any other bootstrap module. This reduces binary -//! size and improves compilation time by minimizing linking time. +//! +//! 1. It is part of the `utils` module and used in other parts of bootstrap. +//! 2. It is embedded inside bootstrap shims to avoid a dependency on the bootstrap library. +//! Therefore, this module should never use any other bootstrap module. This reduces binary size +//! and improves compilation time by minimizing linking time. + +// # Note on tests +// +// If we were to declare a tests submodule here, the shim binaries that include this module via +// `#[path]` would fail to find it, which breaks `./x check bootstrap`. So instead the unit tests +// for this module are in `super::tests::shared_helpers_tests`. #![allow(dead_code)] -#[cfg(test)] -mod tests; - use std::env; use std::ffi::OsString; use std::fs::OpenOptions; @@ -16,10 +20,6 @@ use std::io::Write; use std::process::Command; use std::str::FromStr; -// If we were to declare a tests submodule here, the shim binaries that include this -// module via `#[path]` would fail to find it, which breaks `./x check bootstrap`. -// So instead the unit tests for this module are in `super::tests::shared_helpers_tests`. - /// Returns the environment variable which the dynamic library lookup path /// resides in for this platform. pub fn dylib_path_var() -> &'static str { diff --git a/src/bootstrap/src/utils/shared_helpers/tests.rs b/src/bootstrap/src/utils/shared_helpers/tests.rs deleted file mode 100644 index 559e9f70abd..00000000000 --- a/src/bootstrap/src/utils/shared_helpers/tests.rs +++ /dev/null @@ -1,28 +0,0 @@ -use crate::utils::shared_helpers::parse_value_from_args; - -#[test] -fn test_parse_value_from_args() { - let args = vec![ - "--stage".into(), - "1".into(), - "--version".into(), - "2".into(), - "--target".into(), - "x86_64-unknown-linux".into(), - ]; - - assert_eq!(parse_value_from_args(args.as_slice(), "--stage").unwrap(), "1"); - assert_eq!(parse_value_from_args(args.as_slice(), "--version").unwrap(), "2"); - assert_eq!(parse_value_from_args(args.as_slice(), "--target").unwrap(), "x86_64-unknown-linux"); - assert!(parse_value_from_args(args.as_slice(), "random-key").is_none()); - - let args = vec![ - "app-name".into(), - "--key".into(), - "value".into(), - "random-value".into(), - "--sysroot=/x/y/z".into(), - ]; - assert_eq!(parse_value_from_args(args.as_slice(), "--key").unwrap(), "value"); - assert_eq!(parse_value_from_args(args.as_slice(), "--sysroot").unwrap(), "/x/y/z"); -} diff --git a/src/bootstrap/src/utils/tests/mod.rs b/src/bootstrap/src/utils/tests/mod.rs index ec87e71e0b6..983680b0385 100644 --- a/src/bootstrap/src/utils/tests/mod.rs +++ b/src/bootstrap/src/utils/tests/mod.rs @@ -12,6 +12,10 @@ use crate::{Build, Config, Flags, t}; pub mod git; +// Note: tests for `shared_helpers` is separate here, as otherwise shim binaries that include the +// `shared_helpers` via `#[path]` would fail to find it, breaking `./x check bootstrap`. +mod shared_helpers_tests; + /// Holds temporary state of a bootstrap test. /// Right now it is only used to redirect the build directory of the bootstrap /// invocation, in the future it would be great if we could actually execute diff --git a/src/bootstrap/src/utils/tests/shared_helpers_tests.rs b/src/bootstrap/src/utils/tests/shared_helpers_tests.rs new file mode 100644 index 00000000000..c486e65007e --- /dev/null +++ b/src/bootstrap/src/utils/tests/shared_helpers_tests.rs @@ -0,0 +1,35 @@ +//! The `shared_helpers` module can't have its own tests submodule, because that would cause +//! problems for the shim binaries that include it via `#[path]`, so instead those unit tests live +//! here. +//! +//! To prevent tidy from complaining about this file not being named `tests.rs`, it lives inside a +//! submodule directory named `tests`. + +use crate::utils::shared_helpers::parse_value_from_args; + +#[test] +fn test_parse_value_from_args() { + let args = vec![ + "--stage".into(), + "1".into(), + "--version".into(), + "2".into(), + "--target".into(), + "x86_64-unknown-linux".into(), + ]; + + assert_eq!(parse_value_from_args(args.as_slice(), "--stage").unwrap(), "1"); + assert_eq!(parse_value_from_args(args.as_slice(), "--version").unwrap(), "2"); + assert_eq!(parse_value_from_args(args.as_slice(), "--target").unwrap(), "x86_64-unknown-linux"); + assert!(parse_value_from_args(args.as_slice(), "random-key").is_none()); + + let args = vec![ + "app-name".into(), + "--key".into(), + "value".into(), + "random-value".into(), + "--sysroot=/x/y/z".into(), + ]; + assert_eq!(parse_value_from_args(args.as_slice(), "--key").unwrap(), "value"); + assert_eq!(parse_value_from_args(args.as_slice(), "--sysroot").unwrap(), "/x/y/z"); +} -- cgit 1.4.1-3-g733a5 From 430f4f503c5121c16588d06df450dfa8f0edc717 Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Fri, 25 Jul 2025 19:56:33 +0800 Subject: Check `./x check bootstrap` in `pr-check-1` This check is relatively cheap, and is a quick way to root out breaking check flow of `bootstrap` itself. --- src/ci/docker/host-x86_64/pr-check-1/Dockerfile | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/ci/docker/host-x86_64/pr-check-1/Dockerfile b/src/ci/docker/host-x86_64/pr-check-1/Dockerfile index d3c3cd1b63e..f7d51fba861 100644 --- a/src/ci/docker/host-x86_64/pr-check-1/Dockerfile +++ b/src/ci/docker/host-x86_64/pr-check-1/Dockerfile @@ -40,6 +40,7 @@ COPY host-x86_64/pr-check-1/validate-toolstate.sh /scripts/ # We disable optimized compiler built-ins because that requires a C toolchain for the target. # We also skip the x86_64-unknown-linux-gnu target as it is well-tested by other jobs. ENV SCRIPT \ + python3 ../x.py check bootstrap && \ /scripts/check-default-config-profiles.sh && \ python3 ../x.py build src/tools/build-manifest && \ python3 ../x.py test --stage 0 src/tools/compiletest && \ -- cgit 1.4.1-3-g733a5