Skip to content

test(cdp): e2e-chromium tests silently pass when Chromium is missing or wrong version #118

Description

@aram-devdocs

Found during the e2e test pass for issue #2.

Symptom

crates/plumb-cdp/tests/driver_contract.rs treats CdpError::ChromiumNotFound and CdpError::UnsupportedChromium as a silent pass:

let snap = match driver.snapshot(target(&url)).await {
    Ok(snap) => snap,
    Err(err) if host_missing_chromium(&err) => {
        // Host lacks Chromium 131; treat as a skip. The
        // `print_stderr` workspace lint forbids `eprintln!`, so we
        // surface the skip via a diagnostic write directly.
        let _ = err;
        return Ok(());
    }
    Err(err) => return Err(Box::<dyn std::error::Error>::from(err)),
};

The comment claims to "surface the skip via a diagnostic write" but the implementation just return Ok(()). Cargo reports ok and the test counts toward the green CI total. There is no way to tell from the output whether the test actually exercised Chromium or skipped.

Reproduction

On a host with the wrong Chromium major (e.g. 139 vs the pinned 131):

$ cargo test -p plumb-cdp --release --test driver_contract --features e2e-chromium chromium_driver_captures_static_fixture -- --nocapture
running 1 test
test chromium_driver_captures_static_fixture ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out; finished in 0.69s

0.69s is far too fast to have actually launched Chromium and snapshotted a page — confirming the silent skip. But the cargo report is indistinguishable from a real pass.

Why this matters

This regressed real coverage of #15 (DOMSnapshot integration) on every CI run that doesn't have Chromium 131 pre-installed, and on every developer machine running modern Chrome. Phase 1's "3x byte-diff determinism on the new rules" gate effectively passed without ever driving Chromium on host machines.

It also masked #117 until I tried the CLI manually.

Suggested fix

Pick one:

  1. Use cargo test's native --ignored mechanism: gate the e2e tests with #[ignore = \"requires Chromium <pin>\"] and run them in CI only after explicit Chromium provisioning.
  2. Hard-fail when Chromium isn't usable if the test was selected explicitly. The skip path is fine for cargo test --workspace runs but should be assert!(env::var(\"PLUMB_E2E_CHROMIUM_SKIP\").is_ok(), \"Chromium not available\") so opt-in is explicit.
  3. At minimum, emit a warning to stderr (e.g. via tracing::warn! to stderr) when the skip path is taken, so the test output is not misleading. The print_stderr lint can be scoped to the test file with #[allow(...)].

Parent

Refs #2. Related: #117.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:cdpplumb-cdp crate — Chromium DevTools Protocol driverkind:bugSomething brokenphase-1V0 Phase 1 — real CDP driver + foundational rules

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions