aglinxinyuan opened a new pull request, #7613:
URL: https://github.com/apache/texera/pull/7613

   ### What changes were proposed in this PR?
   
   `ComputingUnitMaster` sat at **6.5% of 93 lines**. It assembles the whole 
service — JWT auth and the session-user value factory, the session handler, the 
websocket upgrade filter, the request log, the resource registrations, and the 
recurring cleanup of expired execution results — and almost none of it was 
verified.
   
   Adds 24 tests to the existing spec, taking it to **78.5% of lines** (73/93).
   
   `run()` is driven once against a **real** Dropwizard `Environment` rather 
than a mock. That is not a stylistic choice: 
`WebSocketUpgradeFilter.configureContext` needs a live 
`MutableServletContextHandler`, so the sibling Mockito pattern does not reach 
it. Once run that way the whole method executes outside a server, including 
`scheduleRecurringCallThroughActorSystem`, which needs only the scheduler.
   
   ### Verification
   
   32 mutations applied and reverted, production diff empty each time. Four 
survived on first application; three were fixed and one is reported as 
unpinnable rather than papered over.
   
   Six further mutations were then run independently, chosen for the failure 
modes the build had not aimed at. All six killed the test they should — 
including two "does another collaborator also set this?" probes that deleting  
and the  registration were needed to answer, and a negative-direction bound 
probe on the expiry window.
   
   ### A cross-suite hazard, and what this PR does about it
   
   `run()` repoints the JVM-wide `SqlServer` singleton at 
`StorageConfig.jdbcUrl`, and `initConnection` **closes the pool it replaces**. 
This spec now points the singleton back at its own embedded database in 
`afterAll` before shutting that pool down, so it is never left aimed at 
production storage for whatever runs next.
   
   The residual risk is stated in the spec rather than hidden: amber sets 
neither `Test / fork` nor `Test / parallelExecution := false`, unlike every 
other module that mixes `MockTexeraDB` (`build.sbt:175` sets it, with a comment 
explaining exactly this). A `Tags.limit(Tags.Test, 1)` does not substitute — as 
`common/workflow-core/build.sbt` notes, that bounds sbt task concurrency, not 
ScalaTest's in-JVM distributor. Verified empirically that this spec does not 
disturb its neighbours: `SessionStateSpec` + `WorkflowServiceSpec` pass 11/11 
alone and 45/45 with this suite added.
   
   That build gap looks worth closing on its own, but it is not this PR's to 
make.
   
   ### Deliberately not included
   
   `createAmberRuntime`, `main`, and the `CLEANUP_ALL_EXECUTION_RESULTS` 
branch, all of which need a started runtime.
   
   No production file is touched.
   
   ### Any related issues, documentation, discussions?
   
   Closes #7612
   
   ### How was this PR tested?
   
   ```
   STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly 
org.apache.texera.web.ComputingUnitMasterSpec"
   ```
   
   ```
   [info] Total number of tests run: 34
   [info] Tests: succeeded 34, failed 0, canceled 0, ignored 0, pending 0
   ```
   
   24 new on top of the existing 10. `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]

Reply via email to