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

   ### What changes were proposed in this PR?
   
   `AmberRuntime` had **no spec**. It owns the actor system the rest of amber 
leans on, the serialization extension every message goes through, and the 
scheduling helpers services use for recurring work.
   
   Adds 15 tests, taking the file from **12.20% to 65.85% of lines** (5/41 → 
27/41, branches 1/8 → 5/8).
   
   Nothing needs a cluster: `startActorWorker(None)` builds its whole system 
with artery on port 0 in under 100 ms, makes no network call, and tears down 
again — so the worker startup path is drivable end to end.
   
   Covered: the serde bootstrap and its memoization, extending an installed 
system rather than replacing it, both scheduling helpers including cancellation 
and the recurring delay, the seed-node URI and master address, the artery host 
binding, the config fallback chain, and the two child actors with their 
dead-letter subscription.
   
   ### Cross-suite safety
   
   This matters more than the coverage number. amber sets neither `Test / fork` 
nor `Test / parallelExecution := false`, so every suite shares one JVM — and 
this file *is* the global state others depend on. The spec saves and restores 
`_actorSystem`, `_serde` and `AmberConfig.masterNodeAddr` by reflection, the 
way eight sibling specs already do, and terminates every system it starts.
   
   Verified rather than asserted: run alongside `ClusterListenerSpec` and 
`SessionStateSpec`, **all 21 tests pass**. Review also drove twelve 
runtime-touching suites in one shared JVM with a sentinel system and address 
installed beforehand, and confirmed both were handed back untouched.
   
   ### Verification
   
   21 mutations applied one at a time and reverted, production diff empty each 
time.
   
   **Review then found three survivors, and two of the three are the same 
failure mode:**
   
   | Survivor | Why it passed | Fix |
   |---|---|---|
   | the fallback chain dropping `pekkoConfig` | the assertion read 
`artery.transport == "tcp"`, which is pekko's **own** `reference.conf` default 
— it holds with `pekkoConfig` absent entirely | assert the kryo serializer 
binding, which only `cluster.conf` supplies |
   | the serde bound to a different, same-named system | the test compared 
`system.name == "Amber"`, not identity | capture and compare with `eq` |
   | `ClusterListener` swapped for `DeadLetterMonitorActor` at the same path | 
an `ActorRef` exposes no class, and nothing exchanges a message only one 
answers | **not fixed** — recorded in the spec instead; pinning it would mean 
adding a seam production does not need |
   
   The first of those also exposed a measurement error worth stating: the build 
reported that mutation as *killed*, but the kill came from an earlier assertion 
in the same test failing first (`actor.provider == "cluster"`, which is 
genuine), so the transport line was never evaluated. **A red test is not 
evidence that the assertion you care about pins anything.**
   
   One further mutation survived during the build and was resolved by deleting 
the assertion rather than keeping it: `cluster.conf` already sets 
`canonical.port = 0`, so the production override of it is unobservable.
   
   ### Deliberately not included
   
   `getNodeIpAddress` and `startActorMaster` bind real network interfaces. One 
partial branch remains at the `mainNodeAddress.isDefined` guard, whose true arm 
needs that same address lookup.
   
   No production file is touched.
   
   ### Any related issues, documentation, discussions?
   
   Closes #7662
   
   ### How was this PR tested?
   
   ```
   STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly 
org.apache.texera.amber.engine.common.AmberRuntimeSpec"
   ```
   
   ```
   [info] Total number of tests run: 15
   [info] Tests: succeeded 15, failed 0, canceled 0, ignored 0, pending 0
   ```
   
   `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