feat: start adding os-specific integration tests

.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,70 @@ 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 iputils-ping grep less sed util-linux"
+          - 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
+            echo "Running test: ${test_name}"
+            chmod +x ${test_name}
+            ${test_name} || result=$?
+          done
+
+          exit ${result}

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.

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"

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>,

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,

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: |

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()

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)]

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;