Skip to content

Commit

Permalink
Fix sandboxing of redirection tests (nushell#11407)
Browse files Browse the repository at this point in the history
When running `cargo test --workspace` a file `crates/nu-command/a.txt`
remained which we also saw as an accidential additions in some commits.

Searching for `a.txt` narrowed it down that
`redirection_keep_exit_codes` was not sandboxed in a temporary directory
and created this file.

Went through redirection tests and placed them in a `Playground` to get
sandboxing `dirs` for `nu!(cwd:`.
For those tests where redirection fails and no file should be created
now I added a check that no file is created on accident.


- Sandbox `redirection_keep_exit_codes` test
- Sandbox `no_duplicate_redirection` test
- Check that no redirect file is created on error
- Sandbox `redirection_should_have_a_target` test
  • Loading branch information
sholderbach authored and dmatos2012 committed Feb 20, 2024
1 parent 9cdd88d commit bf598df
Showing 1 changed file with 51 additions and 25 deletions.
76 changes: 51 additions & 25 deletions crates/nu-command/tests/commands/redirection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,14 @@ fn same_target_redirection_with_too_much_stderr_not_hang_nushell() {

#[test]
fn redirection_keep_exit_codes() {
let out = nu!("do -i { nu --testbin fail e> a.txt } | complete | get exit_code");
// needs to use contains "1", because it complete will output `Some(RawStream)`.
assert!(out.out.contains('1'));
Playground::setup("redirection preserves exit code", |dirs, _| {
let out = nu!(
cwd: dirs.test(),
"do -i { nu --testbin fail e> a.txt } | complete | get exit_code"
);
// needs to use contains "1", because it complete will output `Some(RawStream)`.
assert!(out.out.contains('1'));
});
}

#[test]
Expand Down Expand Up @@ -302,24 +307,29 @@ fn separate_redirection_support_variable() {

#[test]
fn redirection_should_have_a_target() {
let scripts = [
"echo asdf o+e>",
"echo asdf o>",
"echo asdf e>",
"echo asdf o> e>",
"echo asdf o> tmp.txt e>",
"echo asdf o> e> tmp.txt",
"echo asdf o> | ignore",
"echo asdf o>; echo asdf",
];
for code in scripts {
let actual = nu!(code);
assert!(
actual.err.contains("expected redirection target",),
"should be error, code: {}",
code
);
}
Playground::setup("redirection_should_have_a_target", |dirs, _| {
let scripts = [
"echo asdf o+e>",
"echo asdf o>",
"echo asdf e>",
"echo asdf o> e>",
"echo asdf o> tmp.txt e>",
"echo asdf o> e> tmp.txt",
"echo asdf o> | ignore",
"echo asdf o>; echo asdf",
];
for code in scripts {
let actual = nu!(cwd: dirs.test(), code);
assert!(
actual.err.contains("expected redirection target",),
"should be error, code: {code}",
);
assert!(
!dirs.test().join("tmp.txt").exists(),
"No file should be created on error: {code}",
);
}
});
}

#[test]
Expand Down Expand Up @@ -352,8 +362,24 @@ fn redirection_with_pipe() {

#[test]
fn no_duplicate_redirection() {
let actual = nu!("echo 3 o> a.txt o> a.txt");
assert!(actual.err.contains("Redirection can be set only once"));
let actual = nu!("echo 3 e> a.txt e> a.txt");
assert!(actual.err.contains("Redirection can be set only once"));
Playground::setup("redirection does not accept duplicate", |dirs, _| {
let actual = nu!(
cwd: dirs.test(),
"echo 3 o> a.txt o> a.txt"
);
assert!(actual.err.contains("Redirection can be set only once"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"
);
let actual = nu!(
cwd: dirs.test(),
"echo 3 e> a.txt e> a.txt"
);
assert!(actual.err.contains("Redirection can be set only once"));
assert!(
!dirs.test().join("a.txt").exists(),
"No file should be created on error"
);
});
}

0 comments on commit bf598df

Please sign in to comment.