The GitHub Actions job "Required Checks" on texera.git/main has failed. Run started by GitHub user github-merge-queue[bot] (triggered by github-merge-queue[bot]).
Head commit for run: 649a27d8bce33d8857ce6999dc0919f5b36c89ff / Xinyuan Lin <[email protected]> test(amber): cover the workflow lifecycle manager's deferred clean-up (#7872) ### What changes were proposed in this PR? `WorkflowLifecycleManagerSpec` goes from 3 tests to 11, covering the deferred clean-up of a finished workflow: refreshing the deadline when a later terminal state arrives, postponing while a user is connected, and cancelling the pending deadline when a user reconnects before it fires. Measured with one fresh sbt JVM per run, `rm -rf` on the jacoco dir first, and an identical suite-name filter. The "before" figure was re-measured here from HEAD's 3-test spec rather than taken from the build report. | Metric | Before | After | |---|---|---| | Codecov (fully-covered lines) | 14/28 = 50.0% | **25/28 = 89.3%** | | Missed lines | 3 | **0** | | JaCoCo line-hit | 25/28 | **28/28** | | Branch arms | 19 covered / 21 missed | **33 covered / 7 missed** | | Instructions | 194/340 | **332/340** | ### Verification 28 mutations, **27 killed, 1 survivor.** The first draft reported no survivors. That was false: **fourteen semantic survivors existed** against its 10-test version, found across two adversarial passes. The tests that failed to discriminate did so for an instructive reason — several asserted exact log messages, which only proves the interpolants are not *interchangeable*, not that the values are right. **The survivor, with its price tag:** hardcoding line 66's user-count interpolant to the literal `2` survives 11/11. The declined-clean-up message is read at exactly one user count, and the only path into `cleanUp` with a user present requires holding the manager's monitor. Killing it would cost more than it is worth; stated rather than papered over. ### Where review was wrong, and it is worth recording Two reviewer findings did not survive checking: - One reported a mutation as passing 10/10. **Not reproducible on this tree** — it fails two tests, and one of them was already in the pre-review bundle. - Another described a kill mechanism that does not hold: the mutation neuters only one line, while a *different* line in `refreshDeadline` still produces the asserted effect. The consequence claim attached to it was also too strong — the leak it described needs the reconnecting user to stay connected past a further condition. A third suggestion was implemented but with tolerance added: an exact-boundary comparison between a fired-at instant and a logged deadline crosses pekko's `nanoTime` wheel and the wall clock, so it carries 200ms of slack. And one of my own test names asserted an accident as a requirement — "does not schedule a second clean-up once the first has completed" is a consequence of `cleanUp` never resetting its flag plus pekko's post-fire `Cancellable` semantics, not a designed behaviour. Renamed. ### Cost, stated plainly Suite runtime goes from 13s to **37s**. Three fixtures had to move to longer timeouts (3s and 5s) to break a collision the reviewers found — with a short timeout and a user count of 1, a timeout mutation and a count mutation produce the same observable outcome — plus one new 3.5s test. There is no injectable clock: `cleanUpTimeout` is an `Int` in seconds and the scheduler comes through the `AmberRuntime` singleton, so the wall-clock cost is the price of testing this file without a production seam. ### Deliberately not included Lines 33, 40 and 86 stay partial, so the file cannot exceed 25/28 without a production change — line 33 is the Scala 2.13 lazy-val bitmap re-check inside `logger$lzycompute`, and the others are similar guards. Line 97 (`setCleanUpDeadline(newState.state)`, inside a subscribe lambda) is absent from JaCoCo's tracked set entirely: the report lists exactly 28 lines and 97 is not among them, because `SyntheticFilter` drops the enclosing lambda. Nothing outside `WorkflowLifecycleManager.scala` was pinned — in particular not `ExecutionStateStore`'s replay semantics or `AmberRuntime`'s scheduler. No production file is touched. Both `git diff -- '*/src/main/*'` and `git diff --cached -- '*/src/main/*'` are empty, and the production sha256 is byte-identical to the pre-mutation snapshot. ### Any related issues, documentation, discussions? Closes #7869 ### How was this PR tested? ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.WorkflowLifecycleManagerSpec" ``` ``` [info] Total number of tests run: 11 [info] Tests: succeeded 11, failed 0, canceled 0, ignored 0, pending 0 ``` `Test/scalafmtCheck` passes. ### 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/32622162504 With regards, GitHub Actions via GitBox
