Skip to content

Commit 73e534b

Browse files
committed
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`.
1 parent b56aaec commit 73e534b

File tree

3 files changed

+22
-11
lines changed

3 files changed

+22
-11
lines changed

src/bootstrap/src/utils/shared_helpers.rs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
//! This module serves two purposes:
2-
//! 1. It is part of the `utils` module and used in other parts of bootstrap.
3-
//! 2. It is embedded inside bootstrap shims to avoid a dependency on the bootstrap library.
4-
//! Therefore, this module should never use any other bootstrap module. This reduces binary
5-
//! size and improves compilation time by minimizing linking time.
2+
//!
3+
//! 1. It is part of the `utils` module and used in other parts of bootstrap.
4+
//! 2. It is embedded inside bootstrap shims to avoid a dependency on the bootstrap library.
5+
//! Therefore, this module should never use any other bootstrap module. This reduces binary size
6+
//! and improves compilation time by minimizing linking time.
7+
8+
// # Note on tests
9+
//
10+
// If we were to declare a tests submodule here, the shim binaries that include this module via
11+
// `#[path]` would fail to find it, which breaks `./x check bootstrap`. So instead the unit tests
12+
// for this module are in `super::tests::shared_helpers_tests`.
613

714
#![allow(dead_code)]
815

9-
#[cfg(test)]
10-
mod tests;
11-
1216
use std::env;
1317
use std::ffi::OsString;
1418
use std::fs::OpenOptions;
1519
use std::io::Write;
1620
use std::process::Command;
1721
use std::str::FromStr;
1822

19-
// If we were to declare a tests submodule here, the shim binaries that include this
20-
// module via `#[path]` would fail to find it, which breaks `./x check bootstrap`.
21-
// So instead the unit tests for this module are in `super::tests::shared_helpers_tests`.
22-
2323
/// Returns the environment variable which the dynamic library lookup path
2424
/// resides in for this platform.
2525
pub fn dylib_path_var() -> &'static str {

src/bootstrap/src/utils/tests/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ use crate::{Build, Config, Flags, t};
1212

1313
pub mod git;
1414

15+
// Note: tests for `shared_helpers` is separate here, as otherwise shim binaries that include the
16+
// `shared_helpers` via `#[path]` would fail to find it, breaking `./x check bootstrap`.
17+
mod shared_helpers_tests;
18+
1519
/// Holds temporary state of a bootstrap test.
1620
/// Right now it is only used to redirect the build directory of the bootstrap
1721
/// invocation, in the future it would be great if we could actually execute

src/bootstrap/src/utils/shared_helpers/tests.rs renamed to src/bootstrap/src/utils/tests/shared_helpers_tests.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! The `shared_helpers` module can't have its own tests submodule, because that would cause
2+
//! problems for the shim binaries that include it via `#[path]`, so instead those unit tests live
3+
//! here.
4+
//!
5+
//! To prevent tidy from complaining about this file not being named `tests.rs`, it lives inside a
6+
//! submodule directory named `tests`.
7+
18
use crate::utils::shared_helpers::parse_value_from_args;
29

310
#[test]

0 commit comments

Comments
 (0)