aglinxinyuan opened a new pull request, #8038: URL: https://github.com/apache/texera/pull/8038
### What changes were proposed in this PR? `ComputingUnitMasterSpec` goes from 34 tests to 40, reaching `createAmberRuntime` and the two cleanup `catch` blocks. | Metric | Before | After | |---|---|---| | **Codecov (fully-covered lines)** | 67/93 = 72.0% | **75/93 = 80.6%** | | JaCoCo line-hit | 73/93 | 79/93 | | Branch arms | 24/30 | 26/30 | **+8 fully-covered lines and +2 branch arms.** Attribution is exact: lines 75–80 (`createAmberRuntime`'s body) give +6, and lines 277 and 291 (the two `catch` blocks) give +1 line and +1 arm each. Nothing else moved. **A strict reviewer should count this as 6, not 8, and I would rather say so.** The two lines at 277/291 close a scala-logging `isWarnEnabled` macro guard, and no killing mutation exists for them on author-written code — the untaken arm belongs to the macro, not to anything in this file. The six `createAmberRuntime` lines are real and are mutation-proven. The existing spec was already good; this is an extension, not a rescue. ### What the reviewers found Two adversarial reviewers returned four findings against the first draft. The substance of the repair was **mutation strength, not coverage** — and that is worth being explicit about, because the numbers above did not move: **The two tests added by the repair round contribute zero new lines.** The forwarding test re-executes lines 75–80 that the first `createAmberRuntime` test already covered, and the `URISyntaxException` test executes line 285 (already covered) and by construction never reaches 286–291. They exist to kill mutants the original tests could not, and the coverage delta is identical to the builder's. Two pre-existing tests were also strengthened in place with cause assertions. Ten mutations were run in the final pass, **all killed**, each credited to the single test that failed — every run reported exactly `39 succeeded, 1 failed`, with the failing name read out of `target/test-reports/TEST-*.xml` rather than the sbt log, which never names it. The protocol is auditable: production was snapshotted to a scratch dir first (sha256 `e084b184…`), every revert came from that copy rather than from git, and before each apply a script asserted both that the file was byte-identical to the snapshot and that the anchor occurred exactly once, refusing to write otherwise. After each revert the hash was re-checked and the production diff confirmed empty, so no mutant was ever live during another's compile. **Three mutations are reported as unverified rather than as kills.** Deleting either `try`/`catch`, narrowing `case e: Throwable` to `IOException` in `dropCollections`, and reordering `dropCollections` against `deleteReplayLog` inside `cleanExecutions` are all believed to be killed by the pre-existing tests, but the final pass did not re-run them. They are listed as credited-on-trust, not as verified kills. ### Verification Measured with two scoped sbt-jacoco runs, scoping via a throwaway `Tests.Filter` on the **suite name** (never a `-z` phrase filter), byte-identical on both sides, one fresh sbt JVM per measurement with the jacoco directory wiped between them, counters read per-line out of `jacoco.xml`. The before-state ran the `HEAD` spec restored into place (34/34 green); the after-state ran the final spec (40/40 green). The throwaway scope file was deleted. Cross-suite contamination was checked explicitly, because this file touches the process-wide actor system and amber's suites share one strictly-serial JVM: `ComputingUnitMasterSpec` + `SessionStateSpec` + `WorkflowWebsocketResourceSpec` + `CoordinatorSpec` + `AmberClientSpec` in a single JVM — 70 tests, 5 suites, all green. ### Deliberately not included Lines 102–114 and 185–188 are refused rather than merely hard. `main()` would `System.exit` the shared test JVM, and the cleanup branch sits behind a private `static final boolean` that cannot be flipped from a test. Reaching either needs a production seam. Lines 91, 93, 120 and 181 remain partial and are left that way — they are on the do-not-chase list (scalac-generated and logging-guard arms). No production file is touched. ### Any related issues, documentation, discussions? Closes #8037 ### How was this PR tested? ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.ComputingUnitMasterSpec" ``` ``` [info] Total number of tests run: 40 [info] Tests: succeeded 40, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. ``` `WorkflowExecutionService/Test/scalafmtCheck` and `WorkflowExecutionService/Test/scalafix --check` both pass. Re-run after rebasing onto current `main`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
