aglinxinyuan opened a new pull request, #7592: URL: https://github.com/apache/texera/pull/7592
### What changes were proposed in this PR? `WorkflowResource` sat at **82.6% of lines**, and its residue was not scattered: permission-guard arms, exception paths, and one whole endpoint that had never been called — `cloneWorkflow`. Permission guards are exactly where a silent regression matters, which is why this is worth doing despite the modest line count. Adds 16 tests to the existing spec (no third spec file), taking the file to **100% of lines**. Everything it reaches is database-only, so `MockTexeraDB` suffices — `WorkflowVersionResource.insertVersion` is jOOQ plus Jackson `JsonDiff`, `HubResource.recordClone` is jOOQ, and nothing needs LakeFS, Docker or an engine. ### Verification 22 mutations applied and reverted, production diff confirmed empty each time — the clone's `isPublic` argument, the version-insert ordering, the access-level comparisons, and the exception-wrapping arms among them. **Reviewing my own tests then found three that claimed more than they pinned.** All three are now stated in the spec rather than left to be discovered: | Claim | Reality | What changed | |---|---|---| | "wrap a failure raised inside the transaction" | `assignNewOperatorIds` fails *before* `createWorkflow` inserts, so "no copy was created" holds with or without a transaction — replacing `context.transaction` with a plain block leaves the suite green | renamed to what it pins (the exception wrapping), with the gap recorded | | the delete test covers the cleanup tail | it does not — emptying the collected execution ids leaves the suite green. `LargeBinaryManager` is an S3-backed `object` with no seam, and document cleanup needs Iceberg fixtures this spec lacks | recorded as entered-not-verified. What the test *does* pin was confirmed by mutation: removing the `case NonFatal` arm of the outer catch turns it red, so an undecodable URI really is tolerated rather than aborting the delete | | two assertions in the write-access test | both already hold before `persistWorkflow` is called; its write branch touches only `WORKFLOW` and `WORKFLOW_VERSION` | relabelled as guards; the content and version-count assertions carry the pin | ### A note on the numbers jacoco reports a wide line-versus-branch split here — branch coverage stays low even at 100% lines — because of the synthetic branches the Scala compiler generates for this style of code. The line figure is the meaningful one on this file. No production file is touched. ### Any related issues, documentation, discussions? Closes #7591 ### How was this PR tested? ``` sbt "WorkflowExecutionService/testOnly org.apache.texera.web.resource.dashboard.file.WorkflowResourceSpec" ``` ``` [info] Total number of tests run: 68 [info] Tests: succeeded 68, failed 0, canceled 0, ignored 0, pending 0 ``` 16 new on top of the existing 52. `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) -- 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]
