Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7769a63ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some(matched) = matched { | ||
| self.state = StreamMatchState::Matched(matched.clone()); | ||
| Some(matched) | ||
| } else { | ||
| None |
There was a problem hiding this comment.
observe_byte() now freezes the result on the first regex that matches any prefix of the current line. That changes the old semantics in both runners: previously they evaluated the completed line and gave fail patterns priority, but now an early success like "passed" will win permanently even if the same line later contains "FAILED" before the newline arrives. In runs where success and failure tokens can appear on one status line, this can turn a real failure into a reported success.
Useful? React with 👍 / 👎.
| cmd.stdout(Stdio::piped()); | ||
| cmd.stderr(Stdio::piped()); |
There was a problem hiding this comment.
Piping stderr here is a regression because process_output_stream() only consumes child.stdout; stderr is left unread until wait_with_output() after the loop finishes. For long-running guests that emit enough stderr (for example with QEMU debug flags, firmware warnings, or repeated device errors), the stderr pipe can fill, block QEMU, and hang the run before any success/fail regex is reached.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates runtime output handling to support byte-level pattern matching (allowing success/fail detection before newline boundaries), and wires that into the U-Boot shell runner and QEMU runner.
Changes:
- Add a shared byte-stream output matcher with a post-match drain window.
- Extend
SerialTermto support byte-based receive callbacks and delayed stop (stop_after). - Update QEMU and U-Boot runners to use byte-based matching; adjust existing env-var tests and add QEMU-backed matcher integration tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
ostool/src/sterm/mod.rs |
Adds byte-based callback support and delayed stopping for receive loop. |
ostool/src/run/output_matcher.rs |
Introduces shared byte-stream matcher used by runtime runners. |
ostool/src/run/uboot.rs |
Switches U-Boot interaction to byte-based matching with drain period. |
ostool/src/run/qemu.rs |
Reworks QEMU output processing to stream bytes through the matcher and terminate after matches. |
ostool/src/run/mod.rs |
Registers the new output_matcher module. |
ostool/src/utils.rs |
Makes env-var names in tests more unique to avoid collisions. |
ostool/tests/qemu_byte_stream.rs |
Adds QEMU-backed integration tests for byte-stream matching behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cmd.stdout(Stdio::piped()); | ||
| cmd.stderr(Stdio::piped()); |
| let mut line_buf = line_buf.lock().unwrap(); | ||
| line_buf.push(byte); | ||
| if byte == b'\n' { | ||
| let line = String::from_utf8_lossy(&line_buf).to_string(); | ||
| line_buf.clear(); |
| self.line_buf.push(byte); | ||
|
|
||
| let first_match = match self.state { | ||
| StreamMatchState::Pending => { |
| let line = String::from_utf8_lossy(&self.line_buf); | ||
|
|
||
| let matched = self | ||
| .fail_regex | ||
| .iter() |
| )); | ||
| } | ||
|
|
||
| thread::sleep(Duration::from_millis(100)); |
| use anyhow::{Context, Result, bail}; | ||
| use regex::Regex; | ||
|
|
||
| static PORT: AtomicU32 = AtomicU32::new(11000); |
| struct ByteStreamMatcher { | ||
| success_regex: Vec<Regex>, | ||
| fail_regex: Vec<Regex>, | ||
| history: Vec<u8>, | ||
| outcome: Option<MatchOutcome>, | ||
| tail_deadline: Option<Instant>, | ||
| } |
No description provided.