Skip to content

test: remove redundant unit tests and fold lookup tables #882

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.

Sign up for GitHub

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

Open
Brooooooklyn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

test: remove redundant unit tests and fold lookup tables #882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 44 additions & 49 deletions crates/fspy_shared_unix/src/exec/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,39 +231,52 @@ mod tests {
.and_then(|(_, v)| v.as_ref().map(|v| AsRef::<[u8]>::as_ref(v).to_vec()))
}

#[test]
fn inserts_when_absent() {
let mut envs: Vec<(BString, Option<BString>)> = vec![];
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
}
/// (case, envs before the call, appended `LD_PRELOAD` value, expected value)
type AppendCase<'a> = (&'a str, &'a [(&'a str, Option<&'a str>)], &'a str, &'a [u8]);

#[test]
fn noop_when_equal() {
let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/a.so")))];
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
}

#[test]
fn noop_when_value_is_last_entry() {
let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/user.so:/a.so")))];
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/user.so:/a.so".to_vec()));
}

#[test]
fn appends_with_colon_when_present_and_different() {
let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/user.so")))];
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/user.so:/a.so".to_vec()));
}

#[test]
fn sets_without_leading_colon_when_existing_is_empty() {
let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("")))];
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
fn appends_value_to_the_env_list() {
let cases: &[AppendCase<'_>] = &[
("inserts when absent", &[], "/a.so", b"/a.so"),
("noop when equal", &[("LD_PRELOAD", Some("/a.so"))], "/a.so", b"/a.so"),
(
"noop when value is last entry",
&[("LD_PRELOAD", Some("/user.so:/a.so"))],
"/a.so",
b"/user.so:/a.so",
),
(
"appends with colon when present and different",
&[("LD_PRELOAD", Some("/user.so"))],
"/a.so",
b"/user.so:/a.so",
),
(
"sets without leading colon when existing is empty",
&[("LD_PRELOAD", Some(""))],
"/a.so",
b"/a.so",
),
// `lib/a.so` ends with `/a.so` as bytes, but the preceding byte is
// `b` not `:`, so it must NOT be treated as already-present.
(
"does not false match prefix without preceding colon",
&[("LD_PRELOAD", Some("/lib/a.so"))],
"a.so",
b"/lib/a.so:a.so",
),
// An env var present in the list but with `None` value (name without
// `=`) should be rewritten to `Some(value)`.
("inserts when present with none value", &[("LD_PRELOAD", None)], "/a.so", b"/a.so"),
];
for &(case, existing, value, expected) in cases {
let mut envs: Vec<(BString, Option<BString>)> = existing
.iter()
.map(|&(name, value)| (BString::from(name), value.map(BString::from)))
.collect();
append_path_env(&mut envs, "LD_PRELOAD", value);
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(expected.to_vec()), "{case}");
}
}

#[test]
Expand All @@ -274,22 +287,4 @@ mod tests {
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
}

#[test]
fn does_not_false_match_prefix_without_preceding_colon() {
// `lib/a.so` ends with `/a.so` as bytes, but the preceding byte is
// `b` not `:`, so it must NOT be treated as already-present.
let mut envs = vec![(BString::from("LD_PRELOAD"), Some(BString::from("/lib/a.so")))];
append_path_env(&mut envs, "LD_PRELOAD", "a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/lib/a.so:a.so".to_vec()));
}

#[test]
fn inserts_when_present_with_none_value() {
// An env var present in the list but with `None` value (name without
// `=`) should be rewritten to `Some(value)`.
let mut envs = vec![(BString::from("LD_PRELOAD"), None)];
append_path_env(&mut envs, "LD_PRELOAD", "/a.so");
assert_eq!(env(&envs, b"LD_PRELOAD"), Some(b"/a.so".to_vec()));
}
}
129 changes: 44 additions & 85 deletions crates/fspy_shared_unix/src/exec/which.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ pub fn which<R>(

#[cfg(test)]
mod tests {
use std::cell::RefCell;

use bstr::{B, BStr};

use super::*;
Expand Down Expand Up @@ -117,91 +115,52 @@ mod tests {
}
}

#[test]
fn test_which_found() {
let called = RefCell::new(None);
let file: &BStr = B("foo").as_bstr();
let path: &BStr = B("/bin:/usr/bin").as_bstr();
let access = mock_access(&["/bin/foo"], &[], &[]);
let res = which(file, path, access, |found| {
*called.borrow_mut() = Some(found.to_owned());
Ok(())
});
assert!(res.is_ok());
assert_eq!(called.borrow().as_ref().unwrap(), b"/bin/foo".as_bstr());
}

#[test]
fn test_which_not_found() {
let file: &BStr = B("foo").as_bstr();
let path: &BStr = B("/bin:/usr/bin").as_bstr();
let access = mock_access(&[], &[], &[]);
let res = which(file, path, access, |_| Ok(()));
assert_eq!(res.unwrap_err(), nix::Error::ENOENT);
}

#[test]
fn test_which_eacces() {
let file: &BStr = B("foo").as_bstr();
let path: &BStr = B("/bin:/usr/bin").as_bstr();
let access = mock_access(&[], &["/bin/foo", "/usr/bin/foo"], &[]);
let res = which(file, path, access, |_| Ok(()));
assert_eq!(res.unwrap_err(), nix::Error::EACCES);
}

#[test]
fn test_which_enotdir() {
let file: &BStr = B("foo").as_bstr();
let path: &BStr = B("/usr/bin:/bin").as_bstr();
let access = mock_access(&[], &[], &["/bin/foo"]);
let res = which(file, path, access, |_| Ok(()));
assert_eq!(res.unwrap_err(), nix::Error::ENOTDIR);
}

#[test]
fn test_which_slash_in_file() {
let called = RefCell::new(None);
let file: &BStr = B("/usr/bin/foo").as_bstr();
let path: &BStr = B("").as_bstr();
let access = mock_access(&["/usr/bin/foo"], &[], &[]);
let res = which(file, path, access, |found| {
*called.borrow_mut() = Some(found.to_owned());
Ok(())
});
assert!(res.is_ok());
assert_eq!(called.borrow().as_ref().unwrap(), b"/usr/bin/foo".as_bstr());
}
/// (case, file, PATH, executable paths, EACCES paths, ENOTDIR paths,
/// the executable found or the error); any other path fails with ENOENT.
type WhichCase<'a> = (
&'a str,
&'a [u8],
&'a str,
&'a [&'a str],
&'a [&'a str],
&'a [&'a str],
Result<&'a str, nix::Error>,
);

#[test]
fn test_which_empty_file() {
let file: &BStr = B("").as_bstr();
let path: &BStr = B("/bin:/usr/bin").as_bstr();
let access = mock_access(&[], &[], &[]);
let res = which(file, path, access, |_| Ok(()));
assert_eq!(res.unwrap_err(), nix::Error::ENOENT);
}

#[test]
fn test_which_file_too_long() {
fn test_which() {
let long_name = vec![b'a'; NAME_MAX + 1];
let file: &BStr = B(&long_name).as_bstr();
let path: &BStr = B("/bin:/usr/bin").as_bstr();
let access = mock_access(&[], &[], &[]);
let res = which(file, path, access, |_| Ok(()));
assert_eq!(res.unwrap_err(), nix::Error::ENAMETOOLONG);
}

#[test]
fn test_which_empty_path_entry() {
let called = RefCell::new(None);
let file: &BStr = B("foo").as_bstr();
let path: &BStr = B(":/bin").as_bstr();
let access = mock_access(&["foo"], &[], &[]);
let res = which(file, path, access, |found| {
*called.borrow_mut() = Some(found.to_owned());
Ok(())
});
assert!(res.is_ok());
assert_eq!(called.borrow().as_ref().unwrap(), b"foo".as_bstr());
let cases: &[WhichCase<'_>] = &[
("found", b"foo", "/bin:/usr/bin", &["/bin/foo"], &[], &[], Ok("/bin/foo")),
("not found", b"foo", "/bin:/usr/bin", &[], &[], &[], Err(nix::Error::ENOENT)),
(
"eacces",
b"foo",
"/bin:/usr/bin",
&[],
&["/bin/foo", "/usr/bin/foo"],
&[],
Err(nix::Error::EACCES),
),
("enotdir", b"foo", "/usr/bin:/bin", &[], &[], &["/bin/foo"], Err(nix::Error::ENOTDIR)),
("slash in file", b"/usr/bin/foo", "", &["/usr/bin/foo"], &[], &[], Ok("/usr/bin/foo")),
("empty file", b"", "/bin:/usr/bin", &[], &[], &[], Err(nix::Error::ENOENT)),
(
"file too long",
&long_name,
"/bin:/usr/bin",
&[],
&[],
&[],
Err(nix::Error::ENAMETOOLONG),
),
("empty path entry", b"foo", ":/bin", &["foo"], &[], &[], Ok("foo")),
];
for &(case, file, path, executable, fail_eacces, fail_enotdir, expected) in cases {
let access = mock_access(executable, fail_eacces, fail_enotdir);
let res =
which(B(file).as_bstr(), B(path).as_bstr(), access, |found| Ok(found.to_owned()));
assert_eq!(res, expected.map(|found| B(found).as_bstr().to_owned()), "{case}");
}
}
}
14 changes: 0 additions & 14 deletions crates/vt/src/session/execute/glob.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,20 +309,6 @@ mod tests {
assert!(result.is_empty());
}

#[test]
fn test_file_hashes_are_consistent() {
let (_temp, workspace) = create_test_workspace();
let positive: std::collections::BTreeSet<Str> =
std::iter::once("packages/my-pkg/src/index.ts".into()).collect();
let negative = std::collections::BTreeSet::new();

// Run twice and compare hashes
let result1 = compute_globbed_inputs(&workspace, &positive, &negative).unwrap();
let result2 = compute_globbed_inputs(&workspace, &positive, &negative).unwrap();

assert_eq!(result1, result2);
}

#[test]
fn test_file_hashes_change_with_content() {
let (temp, workspace) = create_test_workspace();
Expand Down
9 changes: 0 additions & 9 deletions crates/vt/src/session/reporter/grouped/writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,6 @@ impl Write for GroupedWriter {
mod tests {
use super::*;

#[test]
fn buffers_output() {
let buffer = Rc::new(RefCell::new(Vec::new()));
let mut writer = GroupedWriter::new(Rc::clone(&buffer));
writer.write_all(b"hello ").unwrap();
writer.write_all(b"world").unwrap();
assert_eq!(&*buffer.borrow(), b"hello world");
}

#[test]
fn shared_buffer_interleaves() {
let buffer = Rc::new(RefCell::new(Vec::new()));
Expand Down
51 changes: 19 additions & 32 deletions crates/vt/src/session/reporter/labeled/writer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,39 +103,26 @@ mod tests {
}
}

#[test]
fn single_complete_line() {
let result = labeled_output("[app] ", &[b"hello\n"]);
assert_eq!(result, b"[app] hello\n");
}

#[test]
fn multiple_lines_in_one_chunk() {
let result = labeled_output("[a] ", &[b"line1\nline2\n"]);
assert_eq!(result, b"[a] line1\n[a] line2\n");
}

#[test]
fn partial_line_flushed_on_drop() {
let result = labeled_output("[x] ", &[b"no newline"]);
assert_eq!(result, b"[x] no newline");
}
/// (case, prefix, chunks written and flushed in order, expected output)
type LabeledCase<'a> = (&'a str, &'a str, &'a [&'a [u8]], &'a [u8]);

#[test]
fn split_across_chunks() {
let result = labeled_output("[p] ", &[b"split ", b"line\n"]);
assert_eq!(result, b"[p] split line\n");
}

#[test]
fn empty_line() {
let result = labeled_output("[e] ", &[b"\n"]);
assert_eq!(result, b"[e] \n");
}

#[test]
fn multiple_flushes_with_partial() {
let result = labeled_output("[m] ", &[b"a\nb", b"c\n"]);
assert_eq!(result, b"[m] a\n[m] bc\n");
fn prefixes_each_line() {
let cases: &[LabeledCase<'_>] = &[
("single complete line", "[app] ", &[b"hello\n"], b"[app] hello\n"),
(
"multiple lines in one chunk",
"[a] ",
&[b"line1\nline2\n"],
b"[a] line1\n[a] line2\n",
),
("partial line flushed on drop", "[x] ", &[b"no newline"], b"[x] no newline"),
("split across chunks", "[p] ", &[b"split ", b"line\n"], b"[p] split line\n"),
("empty line", "[e] ", &[b"\n"], b"[e] \n"),
("multiple flushes with partial", "[m] ", &[b"a\nb", b"c\n"], b"[m] a\n[m] bc\n"),
];
for &(case, prefix, chunks, expected) in cases {
assert_eq!(labeled_output(prefix, chunks), expected, "{case}");
}
}
}
17 changes: 0 additions & 17 deletions crates/vt_glob/src/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,23 +267,6 @@ mod tests {
Ok(())
}

#[test]
fn test_match_ignores_directory_only_patterns() -> Result<(), PathGlobError> {
let patterns = vec![
"build/**", // Match everything under build
"!build/keep/**", // But not under build/keep
];
let ignores = PathGlobSet::new(&patterns)?;
// Directory patterns
assert!(ignores.is_match("build/output.js"));
assert!(ignores.is_match("build/assets/style.css"));

// Negated directory
assert!(!ignores.is_match("build/keep/important.txt"));

Ok(())
}

#[test]
fn test_match_ignores_mixed_patterns() -> Result<(), PathGlobError> {
let patterns = vec![
Expand Down
8 changes: 0 additions & 8 deletions crates/vt_powershell/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,4 @@ mod tests {
let resolved = abs(root.as_path().join("node"));
assert!(find_ps1_sibling(&resolved).is_none());
}

#[test]
fn is_stdin_terminal_is_idempotent() {
// The value depends on how the test runner wires stdin (non-TTY under
// nextest), so assert the cached result is stable rather than a fixed
// value.
assert_eq!(is_stdin_terminal(), is_stdin_terminal());
}
}
Loading
Loading