Repository: reubeno/brush
Pull request #240 (2024-10-25 08:50:53)

feat: implement `|&` extension
Closed: #104 

github-actions[bot]: # Performance Benchmark Report
| Benchmark name                         | Baseline (μs) |  Test/PR (μs) |    Delta (μs) | Delta %         |
| -------------------------------------- | ------------- | ------------- | ------------- | --------------- |
| `expand_one_string                   ` | `    3.42 μs` | `    3.42 μs` | `    0.00 μs` | `⚪  Unchanged` |
| `instantiate_shell                   ` | `   59.92 μs` | `   60.71 μs` | `    0.79 μs` | `⚪  Unchanged` |
| `instantiate_shell_with_init_scripts ` | `31329.18 μs` | `32229.48 μs` | `  900.30 μs` | `🟠 +2.87%    ` |
| `parse_bash_completion               ` | ` 2764.64 μs` | ` 2807.33 μs` | `   42.70 μs` | `⚪  Unchanged` |
| `parse_sample_script                 ` | `    4.25 μs` | `    4.21 μs` | `   -0.05 μs` | `⚪  Unchanged` |
| `run_echo_builtin_command            ` | `   90.53 μs` | `   91.53 μs` | `    1.00 μs` | `⚪  Unchanged` |
| `run_one_builtin_command             ` | `  109.18 μs` | `  110.30 μs` | `    1.12 μs` | `⚪  Unchanged` |
| `run_one_external_command            ` | ` 1985.50 μs` | ` 1932.75 μs` | `  -52.76 μs` | `🟢 -2.66%    ` |
| `run_one_external_command_directly   ` | ` 1014.51 μs` | `  998.24 μs` | `  -16.27 μs` | `⚪  Unchanged` |

# Code Coverage Report: Only Changed Files listed

| Package | Base Coverage | New Coverage | Difference |
| ------- | ------------- | ------------ | ---------- |
| brush-core/src/jobs.rs | 🔴 37.23% | 🔴 42.42% | 🟢 5.19% |
| brush-parser/src/parser.rs | 🟢 99.75% | 🟢 99.76% | 🟢 0.01% |
| **Overall Coverage** | **🟢 77.01%** | **🟢 77.15%** | **🟢 0.14%** |

_Minimum allowed coverage is_ `70%`_, this run produced_ `77.15%`
<!-- Sticky Pull Request Comment -->

Patch:
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index bfa8204..94b9dd5 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -93,7 +93,7 @@ jobs:
 
       - name: "Build (native)"
         if: ${{ matrix.target == '' }}
-        run: cargo build --release
+        run: cargo build --release --all-targets
 
       - name: "Build (cross)"
         if: ${{ matrix.target != '' }}
@@ -105,6 +105,15 @@ jobs:
           name: binaries-${{ matrix.arch }}-${{ matrix.os }}
           path: target/${{ matrix.target }}/release/${{ matrix.binary_name }}
 
+      - name: "Upload integration test binaries"
+        if: ${{ matrix.target == '' }}
+        uses: actions/upload-artifact@v4
+        with:
+          name: integration-tests-${{ matrix.arch }}-${{ matrix.os }}
+          path: |
+            target/${{ matrix.target }}/release/deps/brush_*_tests-*
+            !**/*.d
+
   # Test functional correctness
   test:
     strategy:
@@ -317,3 +326,76 @@ jobs:
             pr/benchmarks.txt
             main/benchmarks.txt
             benchmark-results.md
+
+  # Test release binary on a variety of OS platforms.
+  os-tests:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          # N.B. We don't include Ubuntu because it's already covered by the initial test job.
+          - container: "fedora:latest"
+            description: "Fedora/latest"
+            prereqs_command: "dnf install -y bash-completion iputils grep less sed util-linux"
+          - container: "debian:latest"
+            description: "Debian/latest"
+            prereqs_command: "apt-get update -y && apt-get install -y bash-completion bsdmainutils iputils-ping grep less sed"
+          - container: "archlinux:latest"
+            description: "Arch Linux/latest"
+            prereqs_command: "pacman -Sy --noconfirm bash-completion iputils grep less sed util-linux"
+
+    name: "OS target tests (${{ matrix.description }})"
+    runs-on: ubuntu-latest
+    container: ${{ matrix.container }}
+    needs: build
+    steps:
+      # Checkout sources for YAML-based test cases
+      - name: Checkout
+        uses: actions/checkout@v4
+        with:
+          path: sources
+
+      - name: Download binaries
+        uses: actions/download-artifact@v4
+        with:
+          name: binaries-x86_64-linux
+          path: binaries
+
+      - name: Download integration test binaries
+        uses: actions/download-artifact@v4
+        with:
+          name: integration-tests-x86_64-linux
+          path: binaries
+
+      - name: Setup downloads
+        run: |
+          # N.B. Can't use -o pipefail because it's not supported on Debian.
+          set -eux
+          chmod +x binaries/*
+          ls -l -R sources/brush-shell/tests
+          ls -l binaries
+
+      - name: Install prerequisites
+        if: ${{ matrix.prereqs_command != '' }}
+        run: ${{ matrix.prereqs_command }}
+
+      - name: Run tests
+        run: |
+          export BRUSH_PATH=$PWD/binaries/brush
+          export BRUSH_COMPAT_TEST_CASES=$PWD/sources/brush-shell/tests/cases
+          export BRUSH_VERBOSE=true
+
+          result=0
+          for test_name in binaries/*tests*; do
+            # TODO: Re-enable interactive tests.
+            if [[ ${test_name} == *interactive* ]]; then
+              echo "WARNING: skipping interactive test: ${test_name}"
+              continue
+            fi
+
+            echo "Running test: ${test_name}"
+            chmod +x ${test_name}
+            ${test_name} || result=$?
+          done
+
+          exit ${result}
diff --git a/brush-core/src/shell.rs b/brush-core/src/shell.rs
index d1a4b7d..a8f33c1 100644
--- a/brush-core/src/shell.rs
+++ b/brush-core/src/shell.rs
@@ -135,6 +135,8 @@ pub struct CreateOptions {
     pub no_profile: bool,
     /// Whether to skip sourcing the user's rc file.
     pub no_rc: bool,
+    /// Whether to skip inheriting environment variables from the calling process.
+    pub do_not_inherit_env: bool,
     /// Whether the shell is in POSIX compliance mode.
     pub posix: bool,
     /// Whether to print commands and arguments as they are read.
@@ -210,11 +212,13 @@ impl Shell {
     fn initialize_vars(options: &CreateOptions) -> Result<ShellEnvironment, error::Error> {
         let mut env = ShellEnvironment::new();
 
-        // Seed parameters from environment.
-        for (k, v) in std::env::vars() {
-            let mut var = ShellVariable::new(ShellValue::String(v));
-            var.export();
-            env.set_global(k, var)?;
+        // Seed parameters from environment (unless requested not to do so).
+        if !options.do_not_inherit_env {
+            for (k, v) in std::env::vars() {
+                let mut var = ShellVariable::new(ShellValue::String(v));
+                var.export();
+                env.set_global(k, var)?;
+            }
         }
 
         // Set some additional ones.
@@ -685,11 +689,31 @@ impl Shell {
 
     /// Composes the shell's prompt, applying all appropriate expansions.
     pub async fn compose_prompt(&mut self) -> Result<String, error::Error> {
+        self.prompt_from_var_or_default("PS1", self.default_prompt())
+            .await
+    }
+
+    /// Compose's the shell's alternate-side prompt, applying all appropriate expansions.
+    #[allow(clippy::unused_async)]
+    pub async fn compose_alt_side_prompt(&mut self) -> Result<String, error::Error> {
+        Ok(String::new())
+    }
+
+    /// Composes the shell's continuation prompt.
+    pub async fn compose_continuation_prompt(&mut self) -> Result<String, error::Error> {
+        self.prompt_from_var_or_default("PS2", "> ").await
+    }
+
+    async fn prompt_from_var_or_default(
+        &mut self,
+        var_name: &str,
+        default: &str,
+    ) -> Result<String, error::Error> {
         // Retrieve the spec.
-        let ps1 = self.parameter_or_default("PS1", self.default_prompt());
+        let prompt_spec = self.parameter_or_default(var_name, default);
 
         // Expand it.
-        let formatted_prompt = prompt::expand_prompt(self, ps1.as_ref())?;
+        let formatted_prompt = prompt::expand_prompt(self, prompt_spec.as_ref())?;
 
         // NOTE: We're having difficulty with xterm escape sequences going through rustyline;
         // so we strip them here.
@@ -703,18 +727,6 @@ impl Shell {
         Ok(formatted_prompt)
     }
 
-    /// Compose's the shell's alternate-side prompt, applying all appropriate expansions.
-    #[allow(clippy::unused_async)]
-    pub async fn compose_alt_side_prompt(&mut self) -> Result<String, error::Error> {
-        Ok(String::new())
-    }
-
-    /// Retrieve's the shell's continuation prompt.
-    pub fn continuation_prompt(&self) -> Result<String, error::Error> {
-        // TODO: Evaluate expansion?
-        Ok(self.parameter_or_default("PS2", "> "))
-    }
-
     /// Returns the exit status of the last command executed in this shell.
     pub fn last_result(&self) -> u8 {
         self.last_exit_status
diff --git a/brush-interactive/src/interactive_shell.rs b/brush-interactive/src/interactive_shell.rs
index 20e1e19..3e24390 100644
--- a/brush-interactive/src/interactive_shell.rs
+++ b/brush-interactive/src/interactive_shell.rs
@@ -126,7 +126,7 @@ pub trait InteractiveShell {
             let prompt = InteractivePrompt {
                 prompt: shell_mut.as_mut().compose_prompt().await?,
                 alt_side_prompt: shell_mut.as_mut().compose_alt_side_prompt().await?,
-                continuation_prompt: shell_mut.as_mut().continuation_prompt()?,
+                continuation_prompt: shell_mut.as_mut().compose_continuation_prompt().await?,
             };
 
             drop(shell_mut);
diff --git a/brush-parser/src/parser.rs b/brush-parser/src/parser.rs
index 734d363..5979ce5 100644
--- a/brush-parser/src/parser.rs
+++ b/brush-parser/src/parser.rs
@@ -254,8 +254,22 @@ peg::parser! {
             bang:bang()? seq:pipe_sequence() { ast::Pipeline { bang: bang.is_some(), seq } }
         rule bang() -> bool = specific_word("!") { true }
 
-        rule pipe_sequence() -> Vec<ast::Command> =
-            c:command() ++ (specific_operator("|") linebreak()) { c }
+        pub(crate) rule pipe_sequence() -> Vec<ast::Command> =
+            c:(c:command() r:&pipe_extension_redirection()? {? // check for `|&` without consuming the stream.
+                let mut c = c;
+                if r.is_some() {
+                    add_pipe_extension_redirection(&mut c)?;
+                }
+                Ok(c)
+            }) ++ (pipe_operator() linebreak()) {
+            c
+        }
+        rule pipe_operator() =
+            specific_operator("|") /
+            pipe_extension_redirection()
+
+        rule pipe_extension_redirection() -> &'input Token  =
+            non_posix_extensions_enabled() p:specific_operator("|&") { p }
 
         // N.B. We needed to move the function definition branch up to avoid conflicts with array assignment syntax.
         rule command() -> ast::Command =
@@ -826,6 +840,37 @@ fn parse_assignment_word(word: &str) -> Result<ast::Assignment, &'static str> {
     parse_result.map_err(|_| "not assignment word")
 }
 
+// add `2>&1` to the command if the pipeline is `|&`
+fn add_pipe_extension_redirection(c: &mut ast::Command) -> Result<(), &'static str> {
+    let r = ast::IoRedirect::File(
+        Some(2),
+        ast::IoFileRedirectKind::DuplicateOutput,
+        ast::IoFileRedirectTarget::Fd(1),
+    );
+    match c {
+        ast::Command::Simple(c) => {
+            let r = ast::CommandPrefixOrSuffixItem::IoRedirect(r);
+            if let Some(suffix) = &mut c.suffix {
+                suffix.0.push(r);
+            } else {
+                let v = vec![r];
+                c.suffix = Some(ast::CommandSuffix(v));
+            }
+        }
+        ast::Command::Compound(_, l) => {
+            if let Some(r_list) = l {
+                r_list.0.push(r);
+            } else {
+                let v = vec![r];
+                *l = Some(ast::RedirectList(v));
+            }
+        }
+        ast::Command::ExtendedTest(_) => return Err("|& unimplemented for extended tests"),
+        ast::Command::Function(_) => return Err("|& unimplemented for functions"),
+    };
+    Ok(())
+}
+
 fn parse_array_assignment(
     word: &str,
     elements: &[&String],
@@ -858,9 +903,11 @@ fn parse_array_assignment(
 
 #[cfg(test)]
 mod tests {
+
     use super::*;
     use crate::tokenizer::tokenize_str;
     use anyhow::Result;
+    use assert_matches::assert_matches;
 
     #[test]
     fn parse_case() -> Result<()> {
@@ -911,4 +958,33 @@ esac\
 
         Ok(())
     }
+
+    #[test]
+    fn parse_redirection() -> Result<()> {
+        let input = r"echo |& wc";
+
+        let tokens = tokenize_str(input)?;
+        let seq = super::token_parser::pipe_sequence(
+            &Tokens {
+                tokens: tokens.as_slice(),
+            },
+            &ParserOptions::default(),
+            &SourceInfo::default(),
+        )?;
+
+        assert_eq!(seq.len(), 2);
+        assert_matches!(seq[0], ast::Command::Simple(..));
+        if let ast::Command::Simple(c) = &seq[0] {
+            let c = c.suffix.as_ref().unwrap();
+            assert_matches!(
+                c.0[0],
+                ast::CommandPrefixOrSuffixItem::IoRedirect(ast::IoRedirect::File(
+                    Some(2),
+                    ast::IoFileRedirectKind::DuplicateOutput,
+                    ast::IoFileRedirectTarget::Fd(1)
+                ))
+            )
+        }
+        Ok(())
+    }
 }
diff --git a/brush-parser/src/tokenizer.rs b/brush-parser/src/tokenizer.rs
index 5017047..79ee1d1 100644
--- a/brush-parser/src/tokenizer.rs
+++ b/brush-parser/src/tokenizer.rs
@@ -936,7 +936,7 @@ impl<'a, R: ?Sized + std::io::BufRead> Tokenizer<'a, R> {
 
     fn is_operator(&self, s: &str) -> bool {
         // Handle non-POSIX operators.
-        if !self.options.posix_mode && matches!(s, "<<<" | "&>" | "&>>" | ";;&" | ";&") {
+        if !self.options.posix_mode && matches!(s, "<<<" | "&>" | "&>>" | ";;&" | ";&" | "|&") {
             return true;
         }
 
diff --git a/brush-shell/Cargo.toml b/brush-shell/Cargo.toml
index f4a44f7..8dbd52b 100644
--- a/brush-shell/Cargo.toml
+++ b/brush-shell/Cargo.toml
@@ -42,7 +42,7 @@ async-trait = "0.1.83"
 brush-parser = { version = "^0.2.9", path = "../brush-parser" }
 brush-core = { version = "^0.2.11", path = "../brush-core" }
 cfg-if = "1.0.0"
-clap = { version = "4.5.17", features = ["derive", "wrap_help"] }
+clap = { version = "4.5.17", features = ["derive", "env", "wrap_help"] }
 const_format = "0.2.33"
 git-version = "0.3.9"
 lazy_static = "1.5.0"
diff --git a/brush-shell/src/args.rs b/brush-shell/src/args.rs
index 6e1ddbc..0284ec7 100644
--- a/brush-shell/src/args.rs
+++ b/brush-shell/src/args.rs
@@ -73,6 +73,10 @@ pub struct CommandLineArgs {
     #[clap(long = "norc")]
     pub no_rc: bool,
 
+    /// Don't inherit environment variables from the calling process.
+    #[clap(long = "noenv")]
+    pub do_not_inherit_env: bool,
+
     /// Enable shell option.
     #[clap(short = 'O', value_name = "OPTION")]
     pub enabled_shopt_options: Vec<String>,
diff --git a/brush-shell/src/main.rs b/brush-shell/src/main.rs
index 858fe8d..e3faee8 100644
--- a/brush-shell/src/main.rs
+++ b/brush-shell/src/main.rs
@@ -205,6 +205,7 @@ async fn instantiate_shell(
             no_editing: args.no_editing,
             no_profile: args.no_profile,
             no_rc: args.no_rc,
+            do_not_inherit_env: args.do_not_inherit_env,
             posix: args.posix || args.sh_mode,
             print_commands_and_arguments: args.print_commands_and_arguments,
             read_commands_from_stdin,


Test patch:
diff --git a/brush-shell/tests/cases/builtins/command.yaml b/brush-shell/tests/cases/builtins/command.yaml
index 8bfe949..573e7e0 100644
--- a/brush-shell/tests/cases/builtins/command.yaml
+++ b/brush-shell/tests/cases/builtins/command.yaml
@@ -20,13 +20,26 @@ cases:
 
   - name: "command -v"
     stdin: |
+      echo "PATH: $PATH"
+
+      echo "[echo]"
       command -v echo
-      command -v cat
-      command -v $(command -v cat)
 
+      echo "[non-existent]"
       command -v non-existent || echo "1. Not found"
+
+      echo "[/usr/bin/non-existent]"
       command -v /usr/bin/non-existent || echo "2. Not found"
 
+  - name: "command -v with full paths"
+    skip: true # TODO: investigate why this fails on arch linux
+    stdin: |
+      echo "[cat]"
+      command -v cat
+
+      echo "[\$(command -v cat)]"
+      command -v $(command -v cat)
+
   - name: "command -V"
     ignore_stderr: true
     stdin: |
diff --git a/brush-shell/tests/cases/pipeline.yaml b/brush-shell/tests/cases/pipeline.yaml
index 4a60272..6b7a39d 100644
--- a/brush-shell/tests/cases/pipeline.yaml
+++ b/brush-shell/tests/cases/pipeline.yaml
@@ -36,3 +36,8 @@ cases:
       echo "var: ${var}"
       echo hi | { var=3; cat; }
       echo "var: ${var}"
+
+  - name: "pipeline extension"
+    stdin: |
+      echo -e "hello" |& wc -l
+      cat dfdfgdfgdf |& wc -l
diff --git a/brush-shell/tests/compat_tests.rs b/brush-shell/tests/compat_tests.rs
index d1d4c04..402fdaf 100644
--- a/brush-shell/tests/compat_tests.rs
+++ b/brush-shell/tests/compat_tests.rs
@@ -1,5 +1,9 @@
 //! The test harness for brush shell integration tests.
 
+// Only compile this for Unix-like platforms (Linux, macOS) because they have an oracle to compare
+// against.
+#![cfg(unix)]
+
 use anyhow::{Context, Result};
 use assert_fs::fixture::{FileWriteStr, PathChild};
 use clap::Parser;
@@ -35,6 +39,9 @@ impl TestConfig {
     pub fn for_bash_testing(options: &TestOptions) -> Result<Self> {
         // Check for bash version.
         let bash_version_str = get_bash_version_str(Path::new(&options.bash_path))?;
+        if options.verbose {
+            eprintln!("Detected bash version: {bash_version_str}");
+        }
 
         // Skip rc file and profile for deterministic behavior across systems/distros.
         Ok(Self {
@@ -45,7 +52,7 @@ impl TestConfig {
             },
             oracle_version_str: Some(bash_version_str),
             test_shell: ShellConfig {
-                which: WhichShell::ShellUnderTest(String::from("brush")),
+                which: WhichShell::ShellUnderTest(PathBuf::from(&options.brush_path)),
                 // Disable a few fancy UI options for shells under test.
                 default_args: vec![
                     "--norc".into(),
@@ -65,12 +72,12 @@ impl TestConfig {
         Ok(Self {
             name: String::from(SH_CONFIG_NAME),
             oracle_shell: ShellConfig {
-                which: WhichShell::NamedShell(String::from("sh")),
+                which: WhichShell::NamedShell(PathBuf::from("sh")),
                 default_args: vec![],
             },
             oracle_version_str: None,
             test_shell: ShellConfig {
-                which: WhichShell::ShellUnderTest(String::from("brush")),
+                which: WhichShell::ShellUnderTest(PathBuf::from(&options.brush_path)),
                 // Disable a few fancy UI options for shells under test.
                 default_args: vec![
                     String::from("--sh"),
@@ -85,9 +92,7 @@ impl TestConfig {
 }
 
 #[allow(clippy::too_many_lines)]
-async fn cli_integration_tests(options: TestOptions) -> Result<()> {
-    let dir = env!("CARGO_MANIFEST_DIR");
-
+async fn cli_integration_tests(mut options: TestOptions) -> Result<()> {
     let mut success_count = 0;
     let mut skip_count = 0;
     let mut known_failure_count = 0;
@@ -98,18 +103,46 @@ async fn cli_integration_tests(options: TestOptions) -> Result<()> {
         test: std::time::Duration::default(),
     };
 
-    let mut test_configs = vec![];
+    // Resolve paths.
+    let test_cases_dir = options.test_cases_path.as_deref().map_or_else(
+        || PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/cases"),
+        |p| p.to_owned(),
+    );
+
+    // Resolve path to the shell-under-test.
+    if options.brush_path.is_empty() {
+        options.brush_path = assert_cmd::cargo::cargo_bin("brush")
+            .to_string_lossy()
+            .to_string();
+    }
+    if !Path::new(&options.brush_path).exists() {
+        return Err(anyhow::anyhow!(
+            "brush binary not found: {}",
+            options.brush_path
+        ));
+    }
 
+    // Set up test configs
+    let mut test_configs = vec![];
     if options.should_enable_config(BASH_CONFIG_NAME) {
         test_configs.push(TestConfig::for_bash_testing(&options)?);
     }
-
     if options.should_enable_config(SH_CONFIG_NAME) {
         test_configs.push(TestConfig::for_sh_testing(&options)?);
     }
 
+    // Generate a glob pattern to find all the YAML test case files.
+    let glob_pattern = test_cases_dir
+        .join("**/*.yaml")
+        .to_string_lossy()
+        .to_string();
+
+    if options.verbose {
+        eprintln!("Running test cases: {glob_pattern}");
+    }
+
     // Spawn each test case set separately.
-    for entry in glob::glob(format!("{dir}/tests/cases/**/*.yaml").as_ref()).unwrap() {
+    for entry in glob::glob(glob_pattern.as_ref()).unwrap() {
         let entry = entry.unwrap();
 
         let yaml_file = std::fs::File::open(entry.as_path())?;
@@ -472,8 +505,8 @@ impl Default for ShellInvocation {
 
 #[derive(Clone)]
 enum WhichShell {
-    ShellUnderTest(String),
-    NamedShell(String),
+    ShellUnderTest(PathBuf),
+    NamedShell(PathBuf),
 }
 
 struct TestCaseResult {
@@ -943,10 +976,7 @@ impl TestCase {
                     let target_dir = std::env::var("CARGO_TARGET_DIR")
                         .ok()
                         .map_or_else(default_target_dir, PathBuf::from);
-                    (
-                        std::process::Command::new(assert_cmd::cargo::cargo_bin(name)),
-                        Some(target_dir),
-                    )
+                    (std::process::Command::new(name), Some(target_dir))
                 }
                 WhichShell::NamedShell(name) => (std::process::Command::new(name), None),
             },
@@ -964,6 +994,11 @@ impl TestCase {
         test_cmd.env("PS1", "test$ ");
         // Try to get decent backtraces when problems get hit.
         test_cmd.env("RUST_BACKTRACE", "1");
+        // Hard-code a well-known PATH that isn't dependent on the system's configuration.
+        test_cmd.env(
+            "PATH",
+            "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
+        );
 
         // Set up any env vars needed for collecting coverage data.
         if let Some(coverage_target_dir) = &coverage_target_dir {
@@ -1278,7 +1313,7 @@ struct TestOptions {
     pub display_known_failure_details: bool,
 
     /// Display details regarding successful test cases
-    #[clap(short = 'v', long = "verbose")]
+    #[clap(short = 'v', long = "verbose", env = "BRUSH_VERBOSE")]
     pub verbose: bool,
 
     /// Enable a specific configuration
@@ -1294,8 +1329,16 @@ struct TestOptions {
     pub exact_match: bool,
 
     /// Optionaly specify a non-default path for bash
-    #[clap(long = "bash-path", default_value = "bash")]
-    pub bash_path: String,
+    #[clap(long = "bash-path", default_value = "bash", env = "BASH_PATH")]
+    pub bash_path: PathBuf,
+
+    /// Optionally specify a non-default path for brush
+    #[clap(long = "brush-path", default_value = "", env = "BRUSH_PATH")]
+    pub brush_path: String,
+
+    /// Optionally specify path to test cases
+    #[clap(long = "test-cases-path", env = "BRUSH_COMPAT_TEST_CASES")]
+    pub test_cases_path: Option<PathBuf>,
 
     //
     // Compat-only options
@@ -1421,14 +1464,7 @@ fn get_bash_version_str(bash_path: &Path) -> Result<String> {
 
 fn main() -> Result<()> {
     let unparsed_args: Vec<_> = std::env::args().collect();
-    let mut options = TestOptions::parse_from(unparsed_args);
-
-    // Allow overriding the bash path for invocations where custom arguments
-    // can't be used (because other tests get run too and they don't support
-    // those arguments).
-    if let Ok(value) = std::env::var("BASH_PATH") {
-        options.bash_path = value;
-    }
+    let options = TestOptions::parse_from(unparsed_args);
 
     tokio::runtime::Builder::new_multi_thread()
         .enable_all()
diff --git a/brush-shell/tests/completion_tests.rs b/brush-shell/tests/completion_tests.rs
index bc779cd..7cf98e0 100644
--- a/brush-shell/tests/completion_tests.rs
+++ b/brush-shell/tests/completion_tests.rs
@@ -1,3 +1,5 @@
+//! Completion integration tests for brush shell.
+
 // For now, only compile this for Linux.
 #![cfg(target_os = "linux")]
 #![allow(clippy::panic_in_result_fn)]
diff --git a/brush-shell/tests/interactive_tests.rs b/brush-shell/tests/interactive_tests.rs
index 95e8e30..6a4973f 100644
--- a/brush-shell/tests/interactive_tests.rs
+++ b/brush-shell/tests/interactive_tests.rs
@@ -1,3 +1,7 @@
+//! Interactive integration tests for brush shell
+
+// For now, only compile this for Unix-like platforms (Linux, macOS).
+#![cfg(unix)]
 #![allow(clippy::panic_in_result_fn)]
 
 use anyhow::Context;