The GitHub Actions job "Build and push images" on texera.git/main has failed. Run started by GitHub user bobbai00 (triggered by bobbai00).
Head commit for run: ce0510d32cdf654184a6d3d27ce9da11e2bf4bc6 / Xinyuan Lin <[email protected]> test(amber): cover the PVE websocket endpoint and the missing-interpreter guard (#7847) ### What changes were proposed in this PR? A new spec for `PveWebsocketResource.scala`, which had none, plus one test for `PveManager.deletePackages`' missing-interpreter guard. 43 → 47 tests. | File | Before | After | |---|---|---| | `PveWebsocketResource.scala` | 0/9 lines (0%) | **9/9 (100%)** | **That 9-line figure is the only gain this PR can measure locally, and the PveManager half is CI-only.** Measured before *and* after, `PveManager.scala` is bit-for-bit unchanged locally: 170/228 lines, 154/228 branch-complete, 40/140 branches. The reason is that local Windows coverage of that file is the mirror image of CI's — the interpreter is never found here, so `PveResourceSpec`'s 6 pre-existing failures already take both guards' true arms incidentally. On Linux those are precisely the missed arms. On CI the bundle adds 4 fully-covered lines (453, 454, 525, 527) and completes 2 branch arms (452, 524); since Codecov counts a line with any missed arm as missed, those two arms are themselves 2 more fully-covered lines, so **15 lines on Codecov's metric, 13 of which go missed → covered**. Line 526 stays partial permanently: `logger.error(msg)` expands to LazyLogging's one-sided `if (isErrorEnabled)`. Attribution was measured per spec with suite-name filters, not inferred. `PveWebsocketResourceSpec` owns all 9 lines of the websocket file *and* `PveManager` 453/454 with arm 452; the `deletePackages` test owns 521/522/525/527 with arm 524. The two owned sets do not overlap. ### What adversarial review changed The first draft reported 8 mutations and 0 survivors. Re-derived independently, it was 14 mutations with **4 survivors**, and two of the four were real defects rather than equivalent mutants: - **Setting the pump's done flag to `false` passed every test.** That is not equivalent: with the flag never set, the pump re-enters `queue.take()` after `session.close()` and parks forever, because nothing further is ever enqueued — leaking one global-EC thread per websocket connection. The blind spot was the fixture, which stubbed `isOpen` as always-true and `close()` as a counter, so neither termination condition was observable. Now the fixture counts guard evaluations: the guard reads the session exactly n times for an n-line sentinel-terminated stream, because the (n+1)th pass short-circuits on the flag, so a third read can only happen under the mutant. All three tests now kill it. - **The catch arm's `[ERR] ${e.getMessage}` could be replaced by a constant and survive**, and the `finally` sentinel was only ever exercised on the happy path. A new test passes an empty `packages` list, so `.get(0)` throws inside the `Future` — before `PveManager` is entered at all, needing no fake runner. It pins the catch arm *and* makes "the client always gets a sentinel even when the action blows up" a proven claim rather than a happy-path inference. One new test was **dropped as redundant**, proven by measurement rather than argument: `PveWebsocketResourceSpec` alone already covers the whole line set the dropped `installUserPackages` test owned, and under the mutation that deletes the guard's `return`, `PveResourceSpec`'s failure count *drops* from 6 to 4 — it contributes nothing to that kill. A fixture literal was also deleted: `"packages" -> ["colorama==0.4.6","idna"]` is unobservable, since the guard returns before the list is read. Two version-pinned strings implying a contract that no test constrains. ### Verification 14 mutations, **10 killed, 4 survivors**, every survivor listed. The two repaired survivors were each confirmed *surviving on the unrepaired bundle first*, then confirmed killed after — not assumed. | Mutation | Killed by | |---|---| | pump's `done = true` → `done = false` | all 3 websocket tests, on the guard-evaluation assertion | | catch arm message → a constant | reports the action's own exception and still sends the sentinel | | `finally` sentinel `__DONE__` → `__FINISHED__` | all 3 websocket tests, including the new failure-path one | | `if (line == "__DONE__")` → `!=` | all 3 websocket tests | | unknown-action message → a constant | reports an unknown action and closes on the sentinel | | pveName argument → a literal | routes action=install with the cuid and pveName from the handshake | | `cuid` parse → `0` | same test, distinct assertion | | `deletePackages` guard message → a constant | reports a missing interpreter and stops before the system-package check | | `deletePackages` guard `return List(msg)` → empty list | same test, plus an existing `PveResource` case | | `installUserPackages` guard `return` deleted | the websocket install test **alone** | **The four survivors, stated plainly.** All four are mutant-surviving gaps, not merely uncovered paths: - `while (!done && session.isOpen)` → `while (!done)` survives: every session in this spec is open for the whole exchange, so that conjunct never decides anything. The pump's guard is pinned at **entry only**. Killing it needs a closed-session test whose only available assertion is "nothing was sent", which against a `Future` means a sleep with no event to wait on. (The new guard-evaluation counter does not accidentally cover it — under this mutant `isOpen` is never called at all.) - The missing-`action` default can be replaced with garbage and survive: all three tests supply `action` explicitly. Exercising the default means driving `action=create`, which goes straight to the real process runner — a real venv plus `pip install -r amber/requirements.txt`. - Replacing the parsed package list with `Nil`, and inverting `filter(_.nonEmpty)`, both survive — the whole 8-line parse chain is unobservable behind the guard's early return. ### On the file reaching 100% Worth qualifying rather than leaving as a headline. Those 9 tracked lines are the straight-line handshake prologue. JaCoCo emits **no BRANCH counter element** for this file: the class element contains exactly `onOpen` and `<init>`, with no `$anonfun$onOpen$*` lambdas, because every conditional sits inside a `Future { … }` body that the `SyntheticFilter` drops. Those conditionals are asserted but not counted, and only 2 of the 4 things `action` can dispatch to are asserted at all. ### Reported, not pinned `onOpen` parses `cuid` **outside** the `Future`, so a handshake missing `cuid` or `pveName` throws `NumberFormatException`/NPE straight out of `onOpen` and the client is told nothing — the catch arm inside the `Future` cannot see it. The new failure-path test deliberately keeps `cuid` valid and throws inside the `Future`, so it pins the catch arm without cementing that behaviour. No production file is touched. ### Any related issues, documentation, discussions? Closes #7846 ### How was this PR tested? ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.resource.pythonvirtualenvironment.PveResourceSpec org.apache.texera.web.resource.pythonvirtualenvironment.PveWebsocketResourceSpec" ``` ``` [info] Tests: succeeded 41, failed 6, canceled 1, ignored 0, pending 0 ``` `PveWebsocketResourceSpec` is 3/3 green across 5 runs. The 6 failures are pre-existing on Windows and untouched by this change — they are the `Scripts/python.exe` family, identical by name before and after, and the pre-bundle baseline on this machine is 43 run / 37 succeeded / 6 failed / 1 canceled. The new guard-evaluation assertion carries a bounded 500 ms wait per test, and it is not a race: on correct code the third guard pass short-circuits and never reads the session, so the latch can only be counted down by a mutant. `Test/scalafmtCheck` and `Test/scalafix --check` both pass. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Report URL: https://github.com/apache/texera/actions/runs/32612942482 With regards, GitHub Actions via GitBox
