Repository: reubeno/brush
Pull request #81 (2024-06-15 04:40:27)

fix: allow interrupting read builtin, run pipeline cmds in subshell

github-actions[bot]: ## Test Results
398 tests   383 :white_check_mark:  1m 17s :stopwatch:
 64 suites   15 :zzz:
  8 files      0 :x:

Results for commit c39a8e15.

[test-results]:data:application/gzip;base64,H4sIAPUcbWYC/1XMyw7CIBCF4VdpWLuQS8vUlzF0ZBJiL4bCyvjuTlFbXP7fSc5TUBj9Ki4NnBqx5pBKdIbrlqNLYZm5reXmKW2j7uFX1zUjbgT6oHt4MMl2F3JhZDnv4GNc4ldino/TLf4/P1JdFqgeS9eHuExTSBwCde/Ay7YHpUmp1gCiBNJgcVCDpk5ZNxgi8XoDNv9PyAcBAAA=

github-actions[bot]: # Performance Benchmark Report
| Benchmark name                       | Baseline (ns) |  Test/PR (ns) |    Delta (ns) | Delta % |
| ------------------------------------ | ------------- | ------------- | ------------- | -------
| `expand_one_string                   ` | `      3461 ns` | `      3459 ns` | `        -2 ns` | `🟢 -0.06%` |
| `instantiate_shell                   ` | `     61650 ns` | `     61509 ns` | `      -141 ns` | `🟢 -0.23%` |
| `instantiate_shell_with_init_scripts ` | `  28397353 ns` | `  27553973 ns` | `   -843380 ns` | `🟢 -2.97%` |
| `parse_bash_completion               ` | `   5610438 ns` | `   5560044 ns` | `    -50394 ns` | `🟢 -0.90%` |
| `parse_sample_script                 ` | `      8907 ns` | `      8845 ns` | `       -62 ns` | `🟢 -0.70%` |
| `run_echo_builtin_command            ` | `     94740 ns` | `     93795 ns` | `      -945 ns` | `🟢 -1.00%` |
| `run_one_builtin_command             ` | `    111159 ns` | `    111147 ns` | `       -12 ns` | `🟢 -0.01%` |
| `run_one_external_command            ` | `   1995654 ns` | `   1935568 ns` | `    -60086 ns` | `🟢 -3.01%` |
| `run_one_external_command_directly   ` | `   1008027 ns` | `   1008260 ns` | `      +233 ns` | `🟠 +0.02%` |

# Code Coverage Report

| Package | Base Coverage | New Coverage | Difference |
| ------- | ------------- | ------------ | ---------- |
| /home/runner/work/brush/brush/brush-core/src/arithmetic.rs | 🟢 99.01% | 🟢 99.01% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtin.rs | 🔴 32.12% | 🔴 32.12% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/alias.rs | 🟢 83.87% | 🟢 83.87% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/bg.rs | 🔴 0% | 🔴 0% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/break_.rs | 🟢 90% | 🟢 90% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/builtin_.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/cd.rs | 🟢 88.57% | 🟢 88.57% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/colon.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/command.rs | 🟢 93.26% | 🟢 93.26% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/complete.rs | 🔴 21.48% | 🔴 21.48% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/continue_.rs | 🟢 90% | 🟢 90% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/declare.rs | 🟢 82.83% | 🟢 82.83% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/dirs.rs | 🟢 90.48% | 🟢 90.48% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/dot.rs | 🟢 95.65% | 🟢 95.65% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/echo.rs | 🟢 86.11% | 🟢 86.11% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/enable.rs | 🟢 91.07% | 🟢 91.07% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/eval.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/exec.rs | 🟢 91.43% | 🟢 91.43% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/exit.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/export.rs | 🟢 88.73% | 🟢 88.73% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/false_.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/fg.rs | 🔴 0% | 🔴 0% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/getopts.rs | 🟢 97.7% | 🟢 97.7% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/help.rs | 🟢 92.86% | 🟢 92.86% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/jobs.rs | 🔴 41.86% | 🔴 41.86% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/kill.rs | 🔴 0% | 🔴 0% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/let_.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/mod.rs | 🟢 94.61% | 🟢 94.61% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/popd.rs | 🟢 94.12% | 🟢 94.12% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/printf.rs | 🟢 90.91% | 🟢 90.91% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/pushd.rs | 🟢 91.3% | 🟢 91.3% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/pwd.rs | 🟢 89.47% | 🟢 89.47% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/read.rs | 🟢 96.1% | 🟢 75.81% | 🔴 -20.29% |
| /home/runner/work/brush/brush/brush-core/src/builtins/return_.rs | 🟢 83.33% | 🟢 83.33% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/set.rs | 🟢 76.64% | 🟢 76.64% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/shift.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/shopt.rs | 🟢 88.39% | 🟢 88.39% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/test.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/trap.rs | 🟢 83.15% | 🟢 83.15% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/true_.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/type_.rs | 🟢 90.27% | 🟢 90.27% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/umask.rs | 🔴 0% | 🔴 0% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/unalias.rs | 🔴 0% | 🔴 0% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/unimp.rs | 🔴 0% | 🔴 0% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/unset.rs | 🟢 94.64% | 🟢 94.64% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/builtins/wait.rs | 🟢 85.71% | 🟢 85.71% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/commands.rs | 🟢 92.09% | 🟢 92.09% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/completion.rs | 🔴 42.49% | 🔴 42.49% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/env.rs | 🟢 87.85% | 🟢 87.85% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/error.rs | 🔴 25% | 🔴 25% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/escape.rs | 🔴 14.91% | 🔴 14.91% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/expansion.rs | 🟢 95.95% | 🟢 95.95% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/extendedtests.rs | 🟠 64.85% | 🟠 64.15% | 🔴 -0.7% |
| /home/runner/work/brush/brush/brush-core/src/files.rs | 🟠 66.67% | 🟠 66.67% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/functions.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/interp.rs | 🟢 92.96% | 🟢 93.06% | 🟢 0.1% |
| /home/runner/work/brush/brush/brush-core/src/jobs.rs | 🔴 39.38% | 🔴 41.45% | 🟢 2.07% |
| /home/runner/work/brush/brush/brush-core/src/keywords.rs | 🟢 96.88% | 🟢 96.88% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/namedoptions.rs | 🟠 52.34% | 🟠 52.34% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/openfiles.rs | 🟠 51.82% | 🔴 46.71% | 🔴 -5.11% |
| /home/runner/work/brush/brush/brush-core/src/options.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/patterns.rs | 🟢 96.93% | 🟢 96.93% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/prompt.rs | 🟢 78.48% | 🟢 78.48% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/regex.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/shell.rs | 🟢 76.99% | 🟢 78.61% | 🟢 1.62% |
| /home/runner/work/brush/brush/brush-core/src/tests.rs | 🟠 50% | 🟠 50% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/traps.rs | 🟠 50% | 🟠 50% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/users.rs | 🔴 16.13% | 🔴 16.13% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-core/src/variables.rs | 🟢 80.13% | 🟢 79.91% | 🔴 -0.22% |
| /home/runner/work/brush/brush/brush-interactive/src/interactive_shell.rs | 🟢 75.88% | 🟢 75.88% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/arithmetic.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/ast.rs | 🔴 29.36% | 🔴 29.36% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/error.rs | 🟠 75% | 🟢 83.33% | 🟢 8.33% |
| /home/runner/work/brush/brush/brush-parser/src/parser.rs | 🟢 98.66% | 🟢 98.66% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/pattern.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/prompt.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/test_command.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-parser/src/tokenizer.rs | 🟢 95.56% | 🟢 95.68% | 🟢 0.12% |
| /home/runner/work/brush/brush/brush-parser/src/word.rs | 🟢 100% | 🟢 100% | ⚪ 0% |
| /home/runner/work/brush/brush/brush-shell/src/main.rs | 🟠 71.63% | 🟠 71.63% | ⚪ 0% |
| **Overall Coverage** | **🟢 78.63%** | **🟢 78.46%** | **🔴 -0.17%** |

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

Patch:
diff --git a/brush-core/src/builtins/read.rs b/brush-core/src/builtins/read.rs
index 6a50445..e13cf2d 100644
--- a/brush-core/src/builtins/read.rs
+++ b/brush-core/src/builtins/read.rs
@@ -1,5 +1,6 @@
 use clap::Parser;
-use std::{collections::VecDeque, io::Read};
+use std::collections::VecDeque;
+use std::io::Read;
 
 use crate::{builtin, commands, env, error, openfiles, variables};
 
@@ -72,9 +73,6 @@ impl builtin::Command for ReadCommand {
         if self.raw_mode {
             tracing::debug!("read -r is not implemented");
         }
-        if self.silent {
-            return error::unimp("read -s");
-        }
         if self.timeout_in_seconds.is_some() {
             return error::unimp("read -t");
         }
@@ -82,9 +80,10 @@ impl builtin::Command for ReadCommand {
             return error::unimp("read -u");
         }
 
-        let input_line = read_line(context.stdin());
+        let input_line = self.read_line(context.stdin())?;
         if let Some(input_line) = input_line {
             let mut variable_names: VecDeque<String> = self.variable_names.clone().into();
+            let mut spillover: Option<String> = None;
             for field in input_line.split_ascii_whitespace() {
                 if let Some(variable_name) = variable_names.pop_front() {
                     context.shell.env.update_or_add(
@@ -95,9 +94,26 @@ impl builtin::Command for ReadCommand {
                         env::EnvironmentScope::Global,
                     )?;
                 } else {
-                    return error::unimp("too few variable names");
+                    match &mut spillover {
+                        Some(s) => {
+                            s.push(' ');
+                            s.push_str(field);
+                        }
+                        None => spillover = Some(field.to_owned()),
+                    }
                 }
             }
+
+            if let Some(spillover) = spillover {
+                context.shell.env.update_or_add(
+                    "REPLY",
+                    variables::ShellValueLiteral::Scalar(spillover),
+                    |_| Ok(()),
+                    env::EnvironmentLookup::Anywhere,
+                    env::EnvironmentScope::Global,
+                )?;
+            }
+
             Ok(crate::builtin::ExitCode::Success)
         } else {
             Ok(crate::builtin::ExitCode::Custom(1))
@@ -105,26 +121,63 @@ impl builtin::Command for ReadCommand {
     }
 }
 
-fn read_line(mut file: openfiles::OpenFile) -> Option<String> {
-    let mut line = String::new();
-    let mut buffer = [0; 1]; // 1-byte buffer
+impl ReadCommand {
+    fn read_line(&self, mut file: openfiles::OpenFile) -> Result<Option<String>, error::Error> {
+        let orig_term_attr = file.get_term_attr()?;
+        if let Some(orig_term_attr) = &orig_term_attr {
+            let mut updated_term_attr = orig_term_attr.to_owned();
+
+            updated_term_attr
+                .local_flags
+                .remove(nix::sys::termios::LocalFlags::ICANON);
+            updated_term_attr
+                .local_flags
+                .remove(nix::sys::termios::LocalFlags::ISIG);
+
+            if self.silent {
+                updated_term_attr
+                    .local_flags
+                    .remove(nix::sys::termios::LocalFlags::ECHO);
+            }
+
+            file.set_term_attr(nix::sys::termios::SetArg::TCSANOW, &updated_term_attr)?;
+        }
+
+        let mut line = String::new();
+        let mut buffer = [0; 1]; // 1-byte buffer
+
+        // TODO: Look at ignoring errors here.
+        while let Ok(n) = file.read(&mut buffer) {
+            if n == 0 {
+                break; // EOF reached.
+            }
+
+            let ch = buffer[0] as char;
+
+            if ch == '\x03' {
+                return Ok(None); // Ctrl+C aborts.
+            }
+
+            if ch == '\n' {
+                break; // End of line reached
+            }
+
+            // Ignore other control characters.
+            if ch.is_ascii_control() {
+                continue;
+            }
 
-    // TODO: Look at ignoring errors here.
-    while let Ok(n) = file.read(&mut buffer) {
-        if n == 0 {
-            break; // EOF reached
+            line.push(ch);
         }
-        let ch = buffer[0] as char;
 
-        if ch == '\n' {
-            break; // End of line reached
+        if let Some(orig_term_attr) = &orig_term_attr {
+            file.set_term_attr(nix::sys::termios::SetArg::TCSANOW, orig_term_attr)?;
         }
-        line.push(ch);
-    }
 
-    if line.is_empty() {
-        None
-    } else {
-        Some(line)
+        if line.is_empty() {
+            Ok(None)
+        } else {
+            Ok(Some(line))
+        }
     }
 }
diff --git a/brush-core/src/interp.rs b/brush-core/src/interp.rs
index 67eb448..441e4a1 100644
--- a/brush-core/src/interp.rs
+++ b/brush-core/src/interp.rs
@@ -79,7 +79,7 @@ struct PipelineExecutionContext<'a> {
 
     current_pipeline_index: usize,
     pipeline_len: usize,
-    output_pipes: Vec<os_pipe::PipeReader>,
+    output_pipes: &'a mut Vec<os_pipe::PipeReader>,
 
     params: ExecutionParameters,
 }
@@ -301,20 +301,35 @@ impl Execute for ast::Pipeline {
         shell: &mut Shell,
         params: &ExecutionParameters,
     ) -> Result<ExecutionResult, error::Error> {
-        let mut pipeline_context = PipelineExecutionContext {
-            shell,
-            current_pipeline_index: 0,
-            pipeline_len: self.seq.len(),
-            output_pipes: vec![],
-            params: params.clone(),
-        };
-
+        let pipeline_len = self.seq.len();
+        let mut output_pipes = vec![];
         let mut spawn_results = VecDeque::new();
 
-        for command in &self.seq {
-            let spawn_result = command.execute_in_pipeline(&mut pipeline_context).await?;
+        for (current_pipeline_index, command) in self.seq.iter().enumerate() {
+            // If there's only one command in the pipeline, then we run directly in the current shell.
+            // Otherwise, we spawn a separate subshell for each command in the pipeline.
+            let spawn_result = if pipeline_len > 1 {
+                let mut subshell = shell.clone();
+                let mut pipeline_context = PipelineExecutionContext {
+                    shell: &mut subshell,
+                    current_pipeline_index,
+                    pipeline_len,
+                    output_pipes: &mut output_pipes,
+                    params: params.clone(),
+                };
+                command.execute_in_pipeline(&mut pipeline_context).await?
+            } else {
+                let mut pipeline_context = PipelineExecutionContext {
+                    shell,
+                    current_pipeline_index,
+                    pipeline_len,
+                    output_pipes: &mut output_pipes,
+                    params: params.clone(),
+                };
+                command.execute_in_pipeline(&mut pipeline_context).await?
+            };
+
             spawn_results.push_back(spawn_result);
-            pipeline_context.current_pipeline_index += 1;
         }
 
         let mut result = ExecutionResult::success();
diff --git a/brush-core/src/openfiles.rs b/brush-core/src/openfiles.rs
index 6b7694e..3c20905 100644
--- a/brush-core/src/openfiles.rs
+++ b/brush-core/src/openfiles.rs
@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::io::IsTerminal;
 #[cfg(unix)]
 use std::os::fd::AsFd;
 #[cfg(unix)]
@@ -76,6 +77,55 @@ impl OpenFile {
             OpenFile::HereDocument(_) => error::unimp("as_raw_fd for here doc"),
         }
     }
+
+    pub(crate) fn is_term(&self) -> bool {
+        match self {
+            OpenFile::Stdin => std::io::stdin().is_terminal(),
+            OpenFile::Stdout => std::io::stdout().is_terminal(),
+            OpenFile::Stderr => std::io::stderr().is_terminal(),
+            OpenFile::Null => false,
+            OpenFile::File(f) => f.is_terminal(),
+            OpenFile::PipeReader(_) => false,
+            OpenFile::PipeWriter(_) => false,
+            OpenFile::HereDocument(_) => false,
+        }
+    }
+
+    pub(crate) fn get_term_attr(&self) -> Result<Option<nix::sys::termios::Termios>, error::Error> {
+        if !self.is_term() {
+            return Ok(None);
+        }
+
+        let result = match self {
+            OpenFile::Stdin => Some(nix::sys::termios::tcgetattr(std::io::stdin())?),
+            OpenFile::Stdout => Some(nix::sys::termios::tcgetattr(std::io::stdout())?),
+            OpenFile::Stderr => Some(nix::sys::termios::tcgetattr(std::io::stderr())?),
+            OpenFile::Null => None,
+            OpenFile::File(f) => Some(nix::sys::termios::tcgetattr(f)?),
+            OpenFile::PipeReader(_) => None,
+            OpenFile::PipeWriter(_) => None,
+            OpenFile::HereDocument(_) => None,
+        };
+        Ok(result)
+    }
+
+    pub(crate) fn set_term_attr(
+        &self,
+        action: nix::sys::termios::SetArg,
+        termios: &nix::sys::termios::Termios,
+    ) -> Result<(), error::Error> {
+        match self {
+            OpenFile::Stdin => nix::sys::termios::tcsetattr(std::io::stdin(), action, termios)?,
+            OpenFile::Stdout => nix::sys::termios::tcsetattr(std::io::stdout(), action, termios)?,
+            OpenFile::Stderr => nix::sys::termios::tcsetattr(std::io::stderr(), action, termios)?,
+            OpenFile::Null => (),
+            OpenFile::File(f) => nix::sys::termios::tcsetattr(f, action, termios)?,
+            OpenFile::PipeReader(_) => (),
+            OpenFile::PipeWriter(_) => (),
+            OpenFile::HereDocument(_) => (),
+        }
+        Ok(())
+    }
 }
 
 impl From<OpenFile> for Stdio {


Test patch:
diff --git a/brush-core/src/extendedtests.rs b/brush-core/src/extendedtests.rs
index 1fe5ad7..81c92f6 100644
--- a/brush-core/src/extendedtests.rs
+++ b/brush-core/src/extendedtests.rs
@@ -115,7 +115,15 @@ pub(crate) fn apply_unary_predicate_to_str(
             }
         }
         ast::UnaryPredicate::FdIsOpenTerminal => {
-            error::unimp("unary extended test predicate: FdIsOpenTerminal")
+            if let Ok(fd) = operand.parse::<u32>() {
+                if let Some(open_file) = shell.open_files.files.get(&fd) {
+                    Ok(open_file.is_term())
+                } else {
+                    Ok(false)
+                }
+            } else {
+                Ok(false)
+            }
         }
         ast::UnaryPredicate::FileExistsAndIsSetuid => {
             let path = Path::new(operand);
diff --git a/brush-shell/tests/cases/builtins/read.yaml b/brush-shell/tests/cases/builtins/read.yaml
index b706433..e1f0d8c 100644
--- a/brush-shell/tests/cases/builtins/read.yaml
+++ b/brush-shell/tests/cases/builtins/read.yaml
@@ -17,3 +17,21 @@ cases:
           b
     stdin: |
       (echo a; echo b) | while read name; do echo "Hello, $name"; done
+
+  - name: "read from here string"
+    known_failure: true
+    stdin: |
+      read myvar <<< "hello"
+      echo "myvar: ${myvar}"
+
+  - name: "read from process substitution"
+    known_failure: true
+    stdin: |
+      read myvar < <(echo hello)
+      echo "myvar: ${myvar}"
+
+  - name: "read from command substitution"
+    known_failure: true
+    stdin: |
+      read myvar <<< "$(echo hello)"
+      echo "myvar: ${myvar}"
diff --git a/brush-shell/tests/cases/pipeline.yaml b/brush-shell/tests/cases/pipeline.yaml
index 265ed9d..4a60272 100644
--- a/brush-shell/tests/cases/pipeline.yaml
+++ b/brush-shell/tests/cases/pipeline.yaml
@@ -25,3 +25,14 @@ cases:
           exit $1
     stdin: |
       ./script.sh 10 | ./script.sh 0 | ./script.sh 33
+
+  - name: "Side effects in pipe commands"
+    stdin: |
+      var=0
+      echo "var: ${var}"
+      { var=1; }
+      echo "var: ${var}"
+      { var=2; echo hi; } | cat
+      echo "var: ${var}"
+      echo hi | { var=3; cat; }
+      echo "var: ${var}"