The GitHub Actions job "Required Checks" on texera.git/gh-readonly-queue/main/pr-7678-2b66f1a2d4870ee35cadabafa5eee3350d007698 has succeeded. Run started by GitHub user aglinxinyuan (triggered by aglinxinyuan).
Head commit for run: 0635af174364f428fcf6c1282099e621d606265d / Xinyuan Lin <[email protected]> test(amber): cover the executions resource's export, statistics and history paths (#7678) ### What changes were proposed in this PR? `WorkflowExecutionsResource` sat at **83.5% of 316 lines** behind a 44-test spec, with the residue concentrated in four endpoints that need nothing but the embedded test DB. Tests **44 -> 64**. Lines **264/316 (83.5%) -> 298/316 (94.3%)**, measured with only this spec running so the figure is attributable to it alone. | Endpoint | What is now pinned | |---|---| | `getWorkflowResultDownloadability` | the `is_downloadable` predicate itself, the label format, the set union when two restricted scans meet downstream, and BFS termination on a cyclic graph | | `exportResultToLocal` | the id plumbing through the zip branch over a seeded execution, both allowed roles and two rejected ones, and the `@Consumes` + `@FormParam` contract against the form the frontend actually posts | | `retrieveInteractionHistory` | the replay destination ids themselves, read back from a real two-record log; plus NULL and empty `log_location` | | `retrieveWorkflowRuntimeStatistics` | the warehouse guard's exact message, read access, and an execution id belonging to a different workflow | ### Verification — and a correction worth reading The first pass applied 11 mutations and reported all 11 killed, zero survivors. **That number was misleading.** Four independent reviewers were then asked to *refute* the suite rather than confirm it, and between them proposed 19 distinct mutations. Every one of the 19 was run against the original suite first: **all 19 survived 54/54.** None of the findings could be dismissed on survival. All 19 are now killed, each verified red on its intended test by name (read from the JUnit XML, since the console log does not name tests), one mutation at a time with a checkout revert and an asserted-empty production diff between every run. The lesson generalises beyond this file: *a mutation set written by the same author as the tests tends to probe the lines the tests were built around.* Examples of what survived: | Surviving mutation | Why the suite did not notice | |---|---| | `WarehouseReadGuard.assertReadable(uri)` -> unconditional throw | the only assertion was `contains("byo")`, and `byo` came from the test's own URI | | delete `.and(DATASET.IS_DOWNLOADABLE.eq(false))` | every dataset the spec seeded was already non-downloadable | | ignore the form body entirely, hard-coding the parsed request | no seeded execution, so the ids were unobservable | | collapse the whole replay-log branch to `List()` | three tests asserted only `isEmpty` | | `if (merged != existing)` -> `if (true)` | every fixture graph was acyclic | | drop `ADMIN` from the allowed roles | only `REGULAR` was ever exercised | ### Deliberately not included The Iceberg record mapping and the `WorkflowRuntimeStatistics` constructor it solely feeds (needs a live Iceberg table). One line is executed but uncreditable: the call on it throws, so JaCoCo's probe never fires. Two defects are reported rather than pinned, so neither is cemented as a contract: - `removeAllExecutionFiles` (`:386`) runs its `filterNot` **outside** the per-URI `try` and **after** the DB delete, so one undecodable stored URI deletes the rows and then 500s the caller — contradicting the "best-effort cleanup" comment. This is why no `mock:///` cleanup test appears here. - `exportResultToDataset`'s catch body (`:859-864`) is unreachable: the service catches per operator and nothing after it can throw. Three reviewer suggestions were deliberately not taken, each explained in the spec: a "legal `vfs://` URI passes the guard" companion would reach `IcebergCatalogInstance` against a real Postgres rather than the embedded DB, and two mutations on `exportResultToDataset` need a live Iceberg document to observe. No production file is touched. ### Any related issues, documentation, discussions? Closes #7677 ### How was this PR tested? ``` STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly org.apache.texera.web.resource.dashboard.user.workflow.WorkflowExecutionsResourceSpec" ``` ``` [info] Total number of tests run: 64 [info] Tests: succeeded 64, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. ``` The one case that round-trips real replay records needs `AmberRuntime.serde`, injected by the same reflection pattern `ReplayLogGeneratorSpec` uses, but narrowed twice because amber runs every suite in one JVM: it is scoped to that single case rather than `beforeAll`, and a runtime another suite already initialised is reused as-is rather than swapped out underneath it. Both fields are restored and the system it creates is shut down. `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) Report URL: https://github.com/apache/texera/actions/runs/31865335525 With regards, GitHub Actions via GitBox
