aglinxinyuan opened a new issue, #7844: URL: https://github.com/apache/texera/issues/7844
### Task Summary Three small, unrelated gaps that can be closed together because each is a handful of lines with no spec coverage: | File | Gap | |---|---| | `common/auth/.../auth/UserActivityTracker.scala` | the whole 9-line private `defaultUpsert` (lines 151-160) plus the two fatal-escapes-the-catch arms at 89 and 103 | | `amber/.../engine/common/Utils.scala` | `amberHomePath` and `isAmberHomePath` — 8 lines | | `amber/.../architecture/common/WorkflowActor.scala` | the `preStart` catch, 2 lines visible in CI | The infrastructure for the first one is already available and easy to miss: `common/auth` declares `.dependsOn(DAO % "test->test")` directly in `build.sbt`, commented "reuse MockTexeraDB embedded Postgres in tests", so the embedded Postgres is free — no new module wiring needed. Traps worth knowing before writing anything, because most of them are ways to write a test that passes while asserting nothing: 1. **Do not put a production constant on the right-hand side of the assertion meant to pin it.** `assert(amberHome.getFileName.toString == Utils.AMBER_HOME_FOLDER_NAME)` looks like it pins the folder name and does not: production selects the directory *using* that constant, so mutating it to `"common"` moves both sides together and the test stays green (`./common` also exists at depth 1). Assert the literal, and pin the constant separately. 2. **The `Files.walk(cwd, 2)` depth cannot be pinned from a test.** `amberHomePath` is a `lazy val` reading the process working directory, so one JVM takes one branch, and `isAmberHomePath` is private. Depth 2 → 1 survives because `amber/` is a direct child of the repo root; only depth 0 dies, and it dies through production's own exception rather than through any assertion about depth. Do not mistake the depth-0 kill for a depth pin. 3. **`WorkflowActor.preStart`'s try/catch is behaviourally indistinguishable from having no catch at all.** The exception leaves `preStart` either way, Pekko wraps it in `ActorInitializationException` either way, the default decider Stops either way, and the watcher's `Terminated` arrives either way. Its only contribution is the log line. Statement *order* inside `preStart` is the part that is worth pinning, and it can be done race-free: in the failure case, assert the parent probe receives no `RegisterActorRef`. 4. **`markActive`'s `NonFatal` catches need a checked non-fatal to be pinned.** If every non-fatal thrower in the suite is a `RuntimeException` subclass, then widening `case NonFatal(e)` to `case e: RuntimeException` survives at all three catch sites. There are three (inside the executor task, in `markActive`, in `evictStale`) and it is easy to cover two and silently exempt the third. 5. **A tight wall-clock window will not prove the upsert stamps its claim time.** `defaultUpsert` can discard its `ts` argument entirely and a before/after window still passes, because the single-threaded writer picks the task up inside the clock's own granularity. Proving it needs the queue actually stalled — an exclusive table lock on a raw connection outside the pool the writer borrows from will do it. 6. **Use non-adjacent fixture uids.** With uids 8801/8802, an off-by-one on the written uid redirects one case's write onto the other case's seed row, so a mutation "kill" can really be a test aborting on its own clobbered precondition. ### Task Type - [ ] Refactor / Cleanup - [ ] DevOps / Deployment / CI - [x] Testing / QA - [ ] Documentation - [ ] Performance - [ ] Other -- 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]
