The GitHub Actions job "Benchmarks PR Comment" on texera.git/main has succeeded.
Run started by GitHub user aglinxinyuan (triggered by aglinxinyuan).

Head commit for run:
2d60462a9527e5d551b53a94b7875cacfc8e5018 / Xinyuan Lin <[email protected]>
test(amber): cover the promise handlers, metadata endpoint and actor-ref 
mapping (#7770)

### What changes were proposed in this PR?

Six small amber files, three of which had no spec at all. 12 new tests
across 4 new spec files and 2 extended ones.

| File | Before | After |
|---|---|---|
| `DebugCommandHandler` | 33.3% (1/3) | **100%** (3/3) |
| `ConsoleMessageHandler` | 33.3% (1/3) | **100%** (3/3) |
| `SystemMetadataResource` | **0%** (0/2) | **100%** (2/2) |
| `StartHandler` | 73.9%, 2 missed + 4 partial | **87.0%**, 0 missed |
| `PrepareCheckpointHandler` | 61.9%, 2 missed + 6 partial | **71.4%**,
0 missed |
| `PekkoActorRefMappingService` | 85.7%, 2 missed + 5 partial |
**87.8%**, 0 missed |

**This is 12 lines, and I would rather say so than dress it up.** What
earns it is that every one is live code, three files go from partly- or
wholly-untested to 100%, none of it needs infrastructure, and all 10
mutations die. The `PrepareCheckpointHandler` case also closes the
prepare-to-finalize handshake gap that file's own scaladoc flagged as a
follow-up — it is now driven against a real `WorkflowWorker` via
`TestKit` rather than stopping at the main-thread hand-off.

### Verification

10 mutations, **10 killed, no survivors**. Each was applied alone by a
driver that asserts the anchor occurs exactly once in both directions,
reverted with `git diff -- '*/src/main/*'` confirmed empty afterwards,
and the failing test read by name from `amber/target/test-reports`.

| Mutation | Killed by |
|---|---|
| `DebugCommandHandler` addresses the sender instead of the named worker
| addresses the command to the worker the request names, not to the
sender |
| **exchange** the debug request's two fields | relays the request body
verbatim |
| **exchange** `source` and `title` on the console message | hands the
client the reported message unchanged |
| **exchange** the two ends of the client channel in `sendToClient` |
puts the message on the client channel and nowhere else |
| `SystemMetadataResource` filters out `Filter` operators | hands back
an entry for every operator the workspace can place |
| `SystemMetadataResource` reverses the group order | hands back the
palette's group order |
| `StartHandler`'s reader-thread check `nonEmpty` -> `isEmpty` | two
tests, incl. refuses a worker that is neither a source nor a reader of
materialized input |
| `PekkoActorRefMappingService` registers the id before the parent send
| swallows a failed parent lookup and still asks again for the same id |
| **exchange** the checkpoint's queued-input and output keys | saves the
worker's queued input and starts recording what arrives next |
| drop the recorded-inputs write | same test |

### Four sibling files were assessed and deliberately left alone

Recording the evidence so this is not re-derived:

- **`PekkoActorService`** — its two uncovered lines are the entire
bodies of `sendToSelfOnce` and `ask`, and both have **zero call sites**
anywhere in main, test, bench or scripts. Dead code. (Its other
originally-missed lines are in fact already covered.)
- **`WorkflowMessage`** — `case _ => 200L` is unreachable: the trait is
`sealed` and has a single subtype in the file, so only a `null` argument
could reach the default arm. Its other line is a case-class declaration
whose 28 uncovered branches are scalac-generated
`equals`/`copy`/`productElement`.
- **`RecoveryPayload`** — the classes are already constructed by
`AmberMessageEnvelopesSpec`; the uncovered remainder is purely
scalac-generated members, so nothing hand-written could be deleted to
make a test fail.
- **`ComputingUnitWorker`** — the uncovered lines are `main`'s body, and
`AmberRuntime.startActorWorker` binds a cluster seed and mutates
`AmberConfig.masterNodeAddr`, which is global state in amber's shared
test JVM.

Within the accepted files, the remaining partials are
`logger.info`/`logger.debug` macro guards (which do not execute under
CI's `TEXERA_SERVICE_LOG_LEVEL=WARN`) and `val (a, b) = ...`
tuple-destructuring `MatchError` arms.

### A production fragility, reported and not pinned

`PekkoActorRefMappingService.retrieveActorRef`'s catch block builds its
warning as `s"... parentRef = " + actorService.parent` — it re-reads the
very thing whose failure it is handling. A parent lookup that fails
*persistently* rather than transiently therefore throws out of the
handler that exists to contain it. The new test injects exactly one
failure for that reason, and says so in its scaladoc, so a fix is not
blocked.

No production file is touched.

### Any related issues, documentation, discussions?

Closes #7769

### How was this PR tested?

```
STORAGE_ICEBERG_CATALOG_TYPE=postgres sbt "WorkflowExecutionService/testOnly 
org.apache.texera.amber.engine.architecture.coordinator.promisehandlers.DebugCommandHandlerSpec
 
org.apache.texera.amber.engine.architecture.coordinator.promisehandlers.ConsoleMessageHandlerSpec
 
org.apache.texera.amber.engine.architecture.worker.promisehandlers.StartHandlerSpec
 org.apache.texera.web.resource.SystemMetadataResourceSpec 
org.apache.texera.amber.engine.architecture.common.PekkoActorRefMappingServiceSpec
 
org.apache.texera.amber.engine.architecture.worker.promisehandlers.PrepareCheckpointHandlerSpec"
```

```
[info] Total number of tests run: 20
[info] Tests: succeeded 20, failed 0, canceled 0, ignored 0, pending 0
```

The amber unit suite goes from 1919 to 1931 tests. `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/32225881416

With regards,
GitHub Actions via GitBox

Reply via email to