feat(emulator): add EmulatorRunner abstraction and test-emu command#28
Merged
feat(emulator): add EmulatorRunner abstraction and test-emu command#28
Conversation
…command (#23) Introduce a standalone emulator execution layer with a trait-based abstraction and a dedicated CLI command surface for post-build testing. - Add `EmulatorOutcome` and `EmulatorRunResult` types to fbuild-core - Add `EmulatorRunner` async trait with `QemuRunner` and `Avr8jsRunner` implementations in fbuild-daemon - Add `select_runner()` to auto-detect the right backend from platform/MCU - Add `POST /api/test-emu` daemon endpoint (build + emulator run) - Add `fbuild test-emu` CLI command with --timeout, --expect, --halt-on-success, --halt-on-error, and --emulator flags - Unsupported boards produce explicit errors before building - Monitor semantics reused for pass/fail/crash classification Closes #23
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a build-then-emulate flow: new CLI subcommand and daemon endpoint Changes
Sequence DiagramsequenceDiagram
actor User as CLI User
participant CLI as fbuild CLI
participant Daemon as fbuild Daemon
participant Builder as Build System
participant Runner as EmulatorRunner
participant Monitor as Process Monitor
User->>CLI: fbuild test-emu --project <dir> --expect <pattern>
CLI->>Daemon: POST /api/test-emu (TestEmuRequest)
Daemon->>Daemon: validate project & parse platformio.ini
Daemon->>Daemon: select_runner(platform, board, emulator_override)
Daemon->>Builder: spawn blocking build firmware
Builder-->>Daemon: build outputs & firmware artifacts
Daemon->>Runner: Runner::run(EmulatorRunConfig)
Runner->>Monitor: spawn process, capture stdout/stderr
Monitor->>Monitor: detect pass/fail/crash/timeout
Monitor-->>Runner: MonitorOutcome
Runner-->>Daemon: EmulatorRunResult (outcome, stdout, stderr, exit_code)
Daemon-->>CLI: OperationResponse (success, message, exit_code, streams)
CLI->>User: print results & exit with provided code
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fbuild-core/src/emulator.rs`:
- Around line 37-42: The display text for Self::TimedOut { expect_found } is
misleading when no --expect was provided; change the TimedOut variant to carry
whether an expect pattern was configured (e.g., TimedOut { expect_found: bool,
expect_configured: bool } or TimedOut(Option<bool>)) and update every site that
constructs TimedOut to supply the new flag; then update the Display
implementation (the match arm for Self::TimedOut) to render three distinct
messages: when expect_configured && expect_found => "timed out (expected pattern
was found)", when expect_configured && !expect_found => "timed out (expected
pattern NOT found)", and when !expect_configured => "timed out" (no expect
mention).
In `@crates/fbuild-daemon/src/handlers/emulator.rs`:
- Around line 1461-1469: The handler currently sets exit_code = None and never
threads the child process status through, causing failures to be normalized to
1; update run_qemu_process and run_avr8js_headless to return the child process
exit status (e.g. Option<ExitStatus> or similar), propagate that returned status
into monitor_outcome_to_emulator (call
monitor_outcome_to_emulator(qemu_result.outcome, qemu_result.exit_status)) and
set EmulatorRunResult.exit_code (and the OperationResponse that wraps it) to
that actual status instead of None; apply the same change to the other similar
blocks mentioned (around the other occurrences at the indicated ranges) so
EmulatorOutcome::Crashed can be produced and the CLI sees the real emulator exit
code.
- Around line 1393-1408: The QEMU DIO-only check must run in select_runner()
rather than later in QemuRunner::run() so unsupported flash modes fail fast; add
a pre-build check inside select_runner() that reads board.flash_mode (or falls
back to mcu_config.default_flash_mode()), does a case-insensitive compare
against "dio", and if not equal returns the appropriate Unsupported result (use
EmulatorOutcome::Unsupported with a clear message) instead of selecting
QemuRunner; remove or keep the existing check in QemuRunner::run() as a
redundant guard but ensure select_runner() prevents building for qio/qout
targets.
- Around line 1730-1731: The handler currently holds the project lock by
assigning let _guard = lock.lock().await which keeps the lock for the entire
emulator session; release that guard immediately after the
build/serialization-critical section so the long-running emulator phase does not
block other operations. Locate the ctx.project_lock call and the
lock.lock().await assignment (the _guard variable) in the emulator handler, and
explicitly drop or let the guard go out of scope (e.g., call drop(_guard) or
limit the guard to a narrower block) before starting the emulator/session work
while keeping the lock during the build step only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10789f27-f17f-49d1-8dec-66c19a8c6fbb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
crates/fbuild-cli/src/daemon_client.rscrates/fbuild-cli/src/main.rscrates/fbuild-core/src/emulator.rscrates/fbuild-core/src/lib.rscrates/fbuild-daemon/Cargo.tomlcrates/fbuild-daemon/src/handlers/README.mdcrates/fbuild-daemon/src/handlers/emulator.rscrates/fbuild-daemon/src/handlers/operations.rscrates/fbuild-daemon/src/lib.rscrates/fbuild-daemon/src/main.rscrates/fbuild-daemon/src/models.rs
Implement native SimAVR emulator backend supporting any AVR board that advertises simavr in its debug_tools (Uno, Leonardo, Mega, and 100+ others). SimAVR consumes firmware.elf directly and pipes UART output to stdout for pass/fail/timeout classification via the existing monitor. - Add SimavrRunner struct implementing EmulatorRunner trait - Add find_simavr() with platform-specific install hints (apt/brew/MSYS2) - Update select_runner() to support explicit --emulator simavr and auto-detect simavr for non-ATmega328P AVR boards - ATmega328P still defaults to avr8js (no external binary needed) - Add 11 tests covering process monitoring, select_runner routing, and board compatibility validation
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
crates/fbuild-daemon/src/handlers/emulator.rs (3)
1873-1874:⚠️ Potential issue | 🟠 MajorRelease the project lock before emulator execution.
The guard acquired at Line 1874 lives through the emulator phase, so long
test-emuruns block other project operations unnecessarily.Suggested fix
- let lock = ctx.project_lock(&project_dir); - let _guard = lock.lock().await; - - let build_result = { + let build_result = { + let lock = ctx.project_lock(&project_dir); + let _guard = lock.lock().await; let p = platform; tokio::task::spawn_blocking(move || { let orchestrator = fbuild_build::get_orchestrator(p)?; orchestrator.build(¶ms) }) .await };As per coding guidelines, "fbuild-daemon must provide Axum HTTP/WebSocket server with request processors, device lease manager, and lock manager".
Also applies to: 1911-1918, 1958-1981
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1873 - 1874, The project lock acquired via ctx.project_lock(&project_dir) and held in _guard (result of lock.lock().await) must be released before entering the emulator phase so long-running test-emu doesn't block other operations; refactor to limit the guard's scope or explicitly drop(_guard) immediately after the critical section that requires the lock and before calling the emulator runner, and apply the same change for the other occurrences around the emulator code (the blocks that currently hold _guard through the emulator at the regions corresponding to lines ~1911-1918 and ~1958-1981).
1461-1470:⚠️ Potential issue | 🟠 MajorPropagate real emulator exit codes end-to-end.
Lines 1461/1538/1659 force
exit_code: None, and Line 1984 rewrites failures to1. This drops real process status and weakensCrashedvsFailedclassification fidelity.Suggested fix
struct QemuRunResult { outcome: MonitorOutcome, stdout: String, stderr: String, + exit_code: Option<i32>, } @@ struct Avr8jsRunResult { outcome: MonitorOutcome, stdout: String, stderr: String, + exit_code: Option<i32>, } @@ - Ok(QemuRunResult { + let exit_code = child_exit.and_then(|s| s.code()); + Ok(QemuRunResult { outcome, stdout: stdout_buf, stderr: stderr_buf, + exit_code, }) @@ - Ok(Avr8jsRunResult { + let exit_code = child_exit.and_then(|s| s.code()); + Ok(Avr8jsRunResult { outcome, stdout: stdout_buf, stderr: stderr_buf, + exit_code, }) @@ - let exit_code = None; - let outcome = monitor_outcome_to_emulator(qemu_result.outcome, exit_code); + let exit_code = qemu_result.exit_code; + let outcome = monitor_outcome_to_emulator(qemu_result.outcome, exit_code); @@ - let outcome = monitor_outcome_to_emulator(avr8js_result.outcome, None); + let outcome = monitor_outcome_to_emulator(avr8js_result.outcome, avr8js_result.exit_code); @@ - let outcome = monitor_outcome_to_emulator(result.outcome, None); + let outcome = monitor_outcome_to_emulator(result.outcome, result.exit_code); @@ - let exit_code = if success { 0 } else { 1 }; + let exit_code = emu_result.exit_code.unwrap_or(if success { 0 } else { 1 });Also applies to: 1531-1539, 1652-1660, 1983-1999
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1461 - 1470, The code is discarding the real QEMU exit status by setting exit_code = None and later coercing failures to 1; update the run path to extract and propagate the actual process exit code from the qemu_result returned by run_qemu_process (or whatever field holds the child status) into the local exit_code variable, pass that exit_code into monitor_outcome_to_emulator and into the EmulatorRunResult struct (used at the sites around EmulatorRunResult, monitor_outcome_to_emulator, and qemu_result), and remove any logic that overwrites real codes with a hardcoded 1 so that Crashed vs Failed classification uses the original process status end-to-end.
1681-1698:⚠️ Potential issue | 🟠 MajorFail fast on unsupported QEMU flash modes in
select_runner().
select_runner()still accepts ESP32-S3 targets with non-DIO flash modes, and the unsupported check happens later inQemuRunner::run()(Line 1393+), after build cost is already paid.Suggested fix
pub fn select_runner( @@ ) -> fbuild_core::Result<Box<dyn EmulatorRunner>> { let board = fbuild_config::BoardConfig::from_board_id(board_id, board_overrides)?; + let ensure_qemu_flash_mode = |b: &fbuild_config::BoardConfig| -> fbuild_core::Result<()> { + let mcu_config = fbuild_build::esp32::mcu_config::get_mcu_config(&b.mcu)?; + let effective_flash_mode = b + .flash_mode + .as_deref() + .unwrap_or(mcu_config.default_flash_mode()); + if !effective_flash_mode.eq_ignore_ascii_case("dio") { + return Err(fbuild_core::FbuildError::DeployFailed(format!( + "QEMU requires DIO flash mode; effective mode is '{}'", + effective_flash_mode + ))); + } + Ok(()) + }; @@ "qemu" => { @@ if !board.mcu.eq_ignore_ascii_case("esp32s3") { @@ } + ensure_qemu_flash_mode(&board)?; Ok(Box::new(QemuRunner::new( project_dir.to_path_buf(), env_name.to_string(), board, ))) } @@ fbuild_core::Platform::Espressif32 => { if board.mcu.eq_ignore_ascii_case("esp32s3") { + ensure_qemu_flash_mode(&board)?; Ok(Box::new(QemuRunner::new( project_dir.to_path_buf(), env_name.to_string(), board, )))Also applies to: 1757-1764
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1681 - 1698, select_runner() accepts ESP32-S3 targets with unsupported flash modes and defers the error to QemuRunner::run(), causing wasted build cost; inside the "qemu" match arm (where QemuRunner::new is returned) add a fast-fail check on the board's flash mode (e.g., board.flash / board.flash_mode) and return Err(fbuild_core::FbuildError::DeployFailed(...)) if it is not the supported mode (case-insensitive compare for "dio" per the QemuRunner::run() logic), mirroring the exact error message used in QemuRunner::run(); apply the same fast-fail check to the other "qemu" branch referenced in the comment so unsupported flash modes are rejected before building.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/fbuild-daemon/src/handlers/emulator.rs`:
- Around line 1635-1653: SimAVR runner reuses run_qemu_process which produces
QEMU-branded messages; update the SimavrRunner call sites so the backend
identity is passed through instead of "QEMU". Modify the call to
run_qemu_process (and/or its RunQemuOptions) from SimavrRunner to provide a
backend label "simavr" and forward that label into
monitor_outcome_to_emulator(result.outcome, ...) rather than None; reference the
SimavrRunner invocation, run_qemu_process, RunQemuOptions, and
monitor_outcome_to_emulator so logs and user-visible strings reflect "simavr"
instead of "QEMU".
---
Duplicate comments:
In `@crates/fbuild-daemon/src/handlers/emulator.rs`:
- Around line 1873-1874: The project lock acquired via
ctx.project_lock(&project_dir) and held in _guard (result of lock.lock().await)
must be released before entering the emulator phase so long-running test-emu
doesn't block other operations; refactor to limit the guard's scope or
explicitly drop(_guard) immediately after the critical section that requires the
lock and before calling the emulator runner, and apply the same change for the
other occurrences around the emulator code (the blocks that currently hold
_guard through the emulator at the regions corresponding to lines ~1911-1918 and
~1958-1981).
- Around line 1461-1470: The code is discarding the real QEMU exit status by
setting exit_code = None and later coercing failures to 1; update the run path
to extract and propagate the actual process exit code from the qemu_result
returned by run_qemu_process (or whatever field holds the child status) into the
local exit_code variable, pass that exit_code into monitor_outcome_to_emulator
and into the EmulatorRunResult struct (used at the sites around
EmulatorRunResult, monitor_outcome_to_emulator, and qemu_result), and remove any
logic that overwrites real codes with a hardcoded 1 so that Crashed vs Failed
classification uses the original process status end-to-end.
- Around line 1681-1698: select_runner() accepts ESP32-S3 targets with
unsupported flash modes and defers the error to QemuRunner::run(), causing
wasted build cost; inside the "qemu" match arm (where QemuRunner::new is
returned) add a fast-fail check on the board's flash mode (e.g., board.flash /
board.flash_mode) and return Err(fbuild_core::FbuildError::DeployFailed(...)) if
it is not the supported mode (case-insensitive compare for "dio" per the
QemuRunner::run() logic), mirroring the exact error message used in
QemuRunner::run(); apply the same fast-fail check to the other "qemu" branch
referenced in the comment so unsupported flash modes are rejected before
building.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6741c2a9-5769-41c8-80bc-61c0c12f92d9
📒 Files selected for processing (1)
crates/fbuild-daemon/src/handlers/emulator.rs
Add structured artifact bundle type for emulator consumption: - EmulatorArtifactBundle with from_build_dir() and from_paths() constructors - RunnerKind enum for per-backend validation (Avr8js, Simavr, QemuEsp32) - validate_for() checks required artifacts exist on disk - All three runners (QemuRunner, Avr8jsRunner, SimavrRunner) validate bundles - Golden tests for outcome classification (passed/failed/crashed/timeout) - Comprehensive bundle tests (serde, scanning, validation, edge cases)
Extend QEMU runner from ESP32-S3-only to also support base ESP32: - Add generic build_qemu_args() that parameterizes machine type and WDT - QemuRunner dynamically names itself based on MCU (QEMU ESP32/ESP32S3) - select_runner() and deploy handler accept both esp32 and esp32s3 - ADC calibration patch only applied for ESP32-S3 (not base ESP32) - Unsupported variants (ESP32-C3, ESP32-S2, etc.) still fail explicitly - 10 new tests: runner selection, auto-detect, MCU validation, args builder
There was a problem hiding this comment.
♻️ Duplicate comments (5)
crates/fbuild-daemon/src/handlers/emulator.rs (4)
1916-1918:⚠️ Potential issue | 🟠 MajorProject lock held during entire emulator phase.
The
_guardfromlock.lock().awaitis held until the handler returns, blocking other operations on the same project during the (potentially long) emulator run. Only the build phase needs serialization.Consider narrowing the lock scope to just the build:
- let lock = ctx.project_lock(&project_dir); - let _guard = lock.lock().await; - - let build_result = { + let build_result = { + let lock = ctx.project_lock(&project_dir); + let _guard = lock.lock().await; let p = platform; tokio::task::spawn_blocking(move || { ... }) .await };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1916 - 1918, The project lock from ctx.project_lock(&project_dir) is held for the whole handler because _guard stays in scope, blocking other operations during the emulator run; acquire the lock only for the build phase by limiting the guard's scope—call lock.lock().await immediately before the build step (using a short block or explicitly dropping the guard) and ensure the guard (the value bound to _guard) is dropped before starting the emulator so the lock is released while the emulator runs.
1487-1496:⚠️ Potential issue | 🟠 MajorEmulator exit code is not preserved end-to-end.
exit_codeis hardcoded toNoneand the handler rewrites failures to exit code1. This prevents the CLI from surfacing the actual emulator exit status and makesEmulatorOutcome::Crashedclassification less useful.Consider threading the child process exit status through
run_qemu_process→QemuRunResult→monitor_outcome_to_emulator→EmulatorRunResult.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1487 - 1496, The code hardcodes exit_code to None; update run_qemu_process to capture the child process exit status and add that field to QemuRunResult (e.g., exit_code: Option<i32>), then change monitor_outcome_to_emulator signature to accept the exit_code and use it when mapping to EmulatorOutcome, and finally return EmulatorRunResult with exit_code: qemu_result.exit_code instead of None; update all call sites and tests accordingly so the emulator exit status is preserved end-to-end.
1411-1427:⚠️ Potential issue | 🟠 MajorDIO flash mode check should happen in
select_runner()for fail-fast behavior.The PR objective states "unsupported boards must fail early with explicit errors" but the DIO-only validation only happens in
QemuRunner::run(), after the build completes. This means an ESP32 board withqioflash mode will incur the full build cost before failing.Consider moving this check to
select_runner()to fail before building.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1411 - 1427, The DIO-only flash-mode check currently in QemuRunner::run() causes late failures after building; move this validation into select_runner() so unsupported flash modes fail fast. In select_runner(), read board.flash_mode (as_deref()/unwrap_or(mcu_config.default_flash_mode())), compute effective_flash_mode, and if it is not equal to "dio" (use eq_ignore_ascii_case), return the same EmulatorRunResult/EmulatorOutcome::Unsupported used in QemuRunner::run() instead of selecting QemuRunner; then remove the duplicate check from QemuRunner::run().
1673-1691:⚠️ Potential issue | 🟡 MinorSimavrRunner reuses
run_qemu_processwhich emits "QEMU exited..." messages.The
SimavrRunnerdelegates torun_qemu_process(), but that function produces QEMU-branded strings like "QEMU exited normally" or "QEMU exited with code X". This leaks incorrect backend identity in user-visible messages.Consider parameterizing the process label in
RunQemuOptions(e.g.,process_label: &str) and passing "simavr" fromSimavrRunner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-daemon/src/handlers/emulator.rs` around lines 1673 - 1691, The SimavrRunner is leaking QEMU-branded messages because it calls run_qemu_process with RunQemuOptions that lack a backend label; add a new field (e.g., process_label: &str or String) to the RunQemuOptions struct, thread that label through run_qemu_process and use it in all user-visible messages ("QEMU exited..." -> "{process_label} exited..."), then update the SimavrRunner call site (the call in emulator.rs that currently constructs RunQemuOptions) to pass "simavr" as the process_label; ensure monitor_outcome_to_emulator and any outcome-to-message helpers use the same label or continue to map outcomes without embedding the hardcoded "QEMU" string.crates/fbuild-core/src/emulator.rs (1)
183-188:⚠️ Potential issue | 🟡 MinorTimeout display text is misleading when no
--expectpattern was configured.When
expect_found: false, the display renders "expected pattern NOT found" even if no--expectflag was ever provided. This can confuse users who didn't configure an expect pattern.Consider either:
- Tracking whether an expect was configured (e.g.,
TimedOut { expect_configured: bool, expect_found: bool })- Simplifying to just "timed out" when
expect_found: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/fbuild-core/src/emulator.rs` around lines 183 - 188, The Display output for the TimedOut variant is misleading when no expect pattern was configured: update the match arm handling Self::TimedOut { expect_found } in the Display impl in emulator.rs so it only mentions the expect pattern when expect_found is true (e.g., "timed out (expected pattern was found)"), and otherwise print a plain "timed out" message; alternatively, if you prefer to track configuration explicitly, extend the TimedOut variant to TimedOut { expect_configured: bool, expect_found: bool } and use expect_configured to decide whether to include the expect-related text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/fbuild-core/src/emulator.rs`:
- Around line 183-188: The Display output for the TimedOut variant is misleading
when no expect pattern was configured: update the match arm handling
Self::TimedOut { expect_found } in the Display impl in emulator.rs so it only
mentions the expect pattern when expect_found is true (e.g., "timed out
(expected pattern was found)"), and otherwise print a plain "timed out" message;
alternatively, if you prefer to track configuration explicitly, extend the
TimedOut variant to TimedOut { expect_configured: bool, expect_found: bool } and
use expect_configured to decide whether to include the expect-related text.
In `@crates/fbuild-daemon/src/handlers/emulator.rs`:
- Around line 1916-1918: The project lock from ctx.project_lock(&project_dir) is
held for the whole handler because _guard stays in scope, blocking other
operations during the emulator run; acquire the lock only for the build phase by
limiting the guard's scope—call lock.lock().await immediately before the build
step (using a short block or explicitly dropping the guard) and ensure the guard
(the value bound to _guard) is dropped before starting the emulator so the lock
is released while the emulator runs.
- Around line 1487-1496: The code hardcodes exit_code to None; update
run_qemu_process to capture the child process exit status and add that field to
QemuRunResult (e.g., exit_code: Option<i32>), then change
monitor_outcome_to_emulator signature to accept the exit_code and use it when
mapping to EmulatorOutcome, and finally return EmulatorRunResult with exit_code:
qemu_result.exit_code instead of None; update all call sites and tests
accordingly so the emulator exit status is preserved end-to-end.
- Around line 1411-1427: The DIO-only flash-mode check currently in
QemuRunner::run() causes late failures after building; move this validation into
select_runner() so unsupported flash modes fail fast. In select_runner(), read
board.flash_mode (as_deref()/unwrap_or(mcu_config.default_flash_mode())),
compute effective_flash_mode, and if it is not equal to "dio" (use
eq_ignore_ascii_case), return the same
EmulatorRunResult/EmulatorOutcome::Unsupported used in QemuRunner::run() instead
of selecting QemuRunner; then remove the duplicate check from QemuRunner::run().
- Around line 1673-1691: The SimavrRunner is leaking QEMU-branded messages
because it calls run_qemu_process with RunQemuOptions that lack a backend label;
add a new field (e.g., process_label: &str or String) to the RunQemuOptions
struct, thread that label through run_qemu_process and use it in all
user-visible messages ("QEMU exited..." -> "{process_label} exited..."), then
update the SimavrRunner call site (the call in emulator.rs that currently
constructs RunQemuOptions) to pass "simavr" as the process_label; ensure
monitor_outcome_to_emulator and any outcome-to-message helpers use the same
label or continue to map outcomes without embedding the hardcoded "QEMU" string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c116e0b3-3e04-43c7-b417-0c0ca869473b
📒 Files selected for processing (3)
crates/fbuild-core/src/emulator.rscrates/fbuild-daemon/src/handlers/emulator.rscrates/fbuild-deploy/src/esp32.rs
- Fix TimedOut display: show "timed out" (not "expected pattern NOT found") when no --expect pattern was configured - Add process_label to RunQemuOptions so SimavrRunner shows "simavr" instead of QEMU-branded messages in user-visible output - Thread real child process exit code through QemuRunResult, Avr8jsRunResult, and into EmulatorRunResult/OperationResponse instead of hardcoding None/0/1 - Scope project lock to the build phase only so long-running emulator sessions don't block other project operations - Add check_qemu_flash_mode() in select_runner() for fail-fast on non-DIO flash modes before paying the build cost
This was referenced Apr 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EmulatorOutcomeandEmulatorRunResulttypes tofbuild-corefor shared emulator result classification (Passed, Failed, Crashed, TimedOut, Unsupported)EmulatorRunnerasync trait infbuild-daemonwithQemuRunner(ESP32-S3) andAvr8jsRunner(ATmega328P) implementationsPOST /api/test-emudaemon endpoint andfbuild test-emuCLI command that builds firmware then runs it in an emulatorTest plan
cargo clippy --workspace --all-targets -- -D warningscleancargo fmt --allcleanfbuild test-emu tests/platform/uno -e uno(requires Node.js)fbuild test-emu tests/platform/esp32s3 -e esp32s3(requires QEMU)Closes #23
Summary by CodeRabbit
New Features
test-emuCLI command and server API to build-and-run firmware in emulators (QEMU, avr8js, simavr) with timeout, halt-on/expect matching, verbose/timestamp controls, backend selection, and detailed run results (outcome, stdout/stderr, exit code). CLI streams output and uses the emulation exit code on failure.Enhancements
Documentation
Tests