Skip to content

Commit bfb06e6

Browse files
authored
Updated unit tests for runfiles (#3352)
This introduces a small re-implementation of https://crates.io/crates/temp-env while keeping the tests free of dependencies. This should make it substantially easier to write meaningful tests for runfiles.
1 parent 5d5286c commit bfb06e6

File tree

1 file changed

+125
-76
lines changed

1 file changed

+125
-76
lines changed

rust/runfiles/runfiles.rs

Lines changed: 125 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -338,96 +338,145 @@ mod test {
338338
use super::*;
339339

340340
use std::ffi::OsStr;
341+
use std::ffi::OsString;
341342
use std::fs::File;
343+
use std::hash::Hash;
342344
use std::io::prelude::*;
345+
use std::sync::{Mutex, OnceLock};
346+
347+
/// A mutex used to guard
348+
static GLOBAL_MUTEX: OnceLock<Mutex<i32>> = OnceLock::new();
349+
350+
/// Mock out environment variables for a given body fo work. Very similar to
351+
/// [temp-env](https://crates.io/crates/temp-env).
352+
fn with_mock_env<K, V, F, R>(kvs: impl AsRef<[(K, Option<V>)]>, closure: F) -> R
353+
where
354+
K: AsRef<OsStr> + Clone + Eq + Hash,
355+
V: AsRef<OsStr> + Clone,
356+
F: FnOnce() -> R,
357+
{
358+
let mtx = GLOBAL_MUTEX.get_or_init(|| Mutex::new(0));
343359

344-
/// Only `RUNFILES_DIR`` is set.
345-
///
346-
/// This test is a part of `test_manifest_based_can_read_data_from_runfiles` as
347-
/// it modifies environment variables.
348-
fn test_env_only_runfiles_dir(test_srcdir: &OsStr, runfiles_manifest_file: &OsStr) {
349-
env::remove_var(TEST_SRCDIR_ENV_VAR);
350-
env::remove_var(MANIFEST_FILE_ENV_VAR);
351-
let r = Runfiles::create().unwrap();
360+
// Ignore poisoning as it's expected to be another test failing an assertion.
361+
let _guard = mtx.lock().unwrap_or_else(|poisoned| poisoned.into_inner());
352362

353-
let d = rlocation!(r, "rules_rust").unwrap();
354-
let f = rlocation!(r, "rules_rust/rust/runfiles/data/sample.txt").unwrap();
355-
assert_eq!(d.join("rust/runfiles/data/sample.txt"), f);
363+
// track the original state of the environment.
364+
let mut old_env = HashMap::new();
356365

357-
let mut f = File::open(f).unwrap();
366+
// Replace or remove requested environment variables.
367+
for (env, val) in kvs.as_ref() {
368+
// Track the original state of the variable.
369+
match std::env::var_os(env) {
370+
Some(v) => old_env.insert(env, Some(v)),
371+
None => old_env.insert(env, None::<OsString>),
372+
};
358373

359-
let mut buffer = String::new();
360-
f.read_to_string(&mut buffer).unwrap();
374+
match val {
375+
Some(v) => std::env::set_var(env, v),
376+
None => std::env::remove_var(env),
377+
}
378+
}
361379

362-
assert_eq!("Example Text!", buffer);
363-
env::set_var(TEST_SRCDIR_ENV_VAR, test_srcdir);
364-
env::set_var(MANIFEST_FILE_ENV_VAR, runfiles_manifest_file);
380+
// Run requested work.
381+
let result = closure();
382+
383+
// Restore original environment
384+
for (env, val) in old_env {
385+
match val {
386+
Some(v) => std::env::set_var(env, v),
387+
None => std::env::remove_var(env),
388+
}
389+
}
390+
391+
result
365392
}
366393

367-
/// Only `TEST_SRCDIR` is set.
368-
///
369-
/// This test is a part of `test_manifest_based_can_read_data_from_runfiles` as
370-
/// it modifies environment variables.
371-
fn test_env_only_test_srcdir(runfiles_dir: &OsStr, runfiles_manifest_file: &OsStr) {
372-
env::remove_var(RUNFILES_DIR_ENV_VAR);
373-
env::remove_var(MANIFEST_FILE_ENV_VAR);
374-
let r = Runfiles::create().unwrap();
375-
376-
let mut f =
377-
File::open(rlocation!(r, "rules_rust/rust/runfiles/data/sample.txt").unwrap()).unwrap();
378-
379-
let mut buffer = String::new();
380-
f.read_to_string(&mut buffer).unwrap();
381-
382-
assert_eq!("Example Text!", buffer);
383-
env::set_var(RUNFILES_DIR_ENV_VAR, runfiles_dir);
384-
env::set_var(MANIFEST_FILE_ENV_VAR, runfiles_manifest_file);
394+
#[test]
395+
fn test_mock_env() {
396+
let original_name = std::env::var("TEST_WORKSPACE").unwrap();
397+
assert!(
398+
!original_name.is_empty(),
399+
"In Bazel tests, `TEST_WORKSPACE` is expected to be populated."
400+
);
401+
402+
let mocked_name = with_mock_env([("TEST_WORKSPACE", Some("foobar"))], || {
403+
std::env::var("TEST_WORKSPACE").unwrap()
404+
});
405+
406+
assert_eq!(mocked_name, "foobar");
407+
assert_eq!(original_name, std::env::var("TEST_WORKSPACE").unwrap());
385408
}
386409

387-
/// Neither `RUNFILES_DIR` or `TEST_SRCDIR` are set
388-
///
389-
/// This test is a part of `test_manifest_based_can_read_data_from_runfiles` as
390-
/// it modifies environment variables.
391-
fn test_env_nothing_set(
392-
test_srcdir: &OsStr,
393-
runfiles_dir: &OsStr,
394-
runfiles_manifest_file: &OsStr,
395-
) {
396-
env::remove_var(RUNFILES_DIR_ENV_VAR);
397-
env::remove_var(TEST_SRCDIR_ENV_VAR);
398-
env::remove_var(MANIFEST_FILE_ENV_VAR);
399-
400-
let r = Runfiles::create().unwrap();
401-
402-
let mut f =
403-
File::open(rlocation!(r, "rules_rust/rust/runfiles/data/sample.txt").unwrap()).unwrap();
404-
405-
let mut buffer = String::new();
406-
f.read_to_string(&mut buffer).unwrap();
407-
408-
assert_eq!("Example Text!", buffer);
409-
410-
env::set_var(TEST_SRCDIR_ENV_VAR, test_srcdir);
411-
env::set_var(RUNFILES_DIR_ENV_VAR, runfiles_dir);
412-
env::set_var(MANIFEST_FILE_ENV_VAR, runfiles_manifest_file);
410+
/// Only `RUNFILES_DIR` is set.
411+
#[test]
412+
fn test_env_only_runfiles_dir() {
413+
with_mock_env(
414+
[
415+
(TEST_SRCDIR_ENV_VAR, None::<&str>),
416+
(MANIFEST_FILE_ENV_VAR, None::<&str>),
417+
],
418+
|| {
419+
let r = Runfiles::create().unwrap();
420+
421+
let d = rlocation!(r, "rules_rust").unwrap();
422+
let f = rlocation!(r, "rules_rust/rust/runfiles/data/sample.txt").unwrap();
423+
assert_eq!(d.join("rust/runfiles/data/sample.txt"), f);
424+
425+
let mut f = File::open(f).unwrap();
426+
427+
let mut buffer = String::new();
428+
f.read_to_string(&mut buffer).unwrap();
429+
430+
assert_eq!("Example Text!", buffer);
431+
},
432+
);
433+
}
434+
435+
/// Only `TEST_SRCDIR` is set.
436+
#[test]
437+
fn test_env_only_test_srcdir() {
438+
with_mock_env(
439+
[
440+
(RUNFILES_DIR_ENV_VAR, None::<&str>),
441+
(MANIFEST_FILE_ENV_VAR, None::<&str>),
442+
],
443+
|| {
444+
let r = Runfiles::create().unwrap();
445+
446+
let mut f =
447+
File::open(rlocation!(r, "rules_rust/rust/runfiles/data/sample.txt").unwrap())
448+
.unwrap();
449+
450+
let mut buffer = String::new();
451+
f.read_to_string(&mut buffer).unwrap();
452+
453+
assert_eq!("Example Text!", buffer);
454+
},
455+
);
413456
}
414457

458+
/// Neither `RUNFILES_DIR` or `TEST_SRCDIR` are set
415459
#[test]
416-
fn test_can_read_data_from_runfiles() {
417-
// We want to run multiple test cases with different environment variables set. Since
418-
// environment variables are global state, we need to ensure the test cases do not run
419-
// concurrently. Rust runs tests in parallel and does not provide an easy way to synchronise
420-
// them, so we run all test cases in the same #[test] function.
421-
422-
let test_srcdir =
423-
env::var_os(TEST_SRCDIR_ENV_VAR).expect("bazel did not provide TEST_SRCDIR");
424-
let runfiles_dir =
425-
env::var_os(RUNFILES_DIR_ENV_VAR).expect("bazel did not provide RUNFILES_DIR");
426-
let runfiles_manifest_file = env::var_os(MANIFEST_FILE_ENV_VAR).unwrap_or("".into());
427-
428-
test_env_only_runfiles_dir(&test_srcdir, &runfiles_manifest_file);
429-
test_env_only_test_srcdir(&runfiles_dir, &runfiles_manifest_file);
430-
test_env_nothing_set(&test_srcdir, &runfiles_dir, &runfiles_manifest_file);
460+
fn test_env_nothing_set() {
461+
with_mock_env(
462+
[
463+
(RUNFILES_DIR_ENV_VAR, None::<&str>),
464+
(TEST_SRCDIR_ENV_VAR, None::<&str>),
465+
(MANIFEST_FILE_ENV_VAR, None::<&str>),
466+
],
467+
|| {
468+
let r = Runfiles::create().unwrap();
469+
470+
let mut f =
471+
File::open(rlocation!(r, "rules_rust/rust/runfiles/data/sample.txt").unwrap())
472+
.unwrap();
473+
474+
let mut buffer = String::new();
475+
f.read_to_string(&mut buffer).unwrap();
476+
477+
assert_eq!("Example Text!", buffer);
478+
},
479+
);
431480
}
432481

433482
#[test]

0 commit comments

Comments
 (0)