The GitHub Actions job "Benchmarks" on texera.git/main has succeeded.
Run started by GitHub user github-merge-queue[bot] (triggered by 
github-merge-queue[bot]).

Head commit for run:
eba468720500ab7a6d786dc4f59282e2c7a6f65f / Xinyuan Lin <[email protected]>
test(amber): cover the replay-log writer, manager and congestion control (#8177)

### What changes were proposed in this PR?

Two new specs for classes that had none, plus one append. 33 tests
across the three suites.

| File | Codecov lines | Newly covered |
|---|---|---|
| `AsyncReplayLogWriter.scala` | 38/44 → **42/44** | 51, 58, 71, 72 |
| `CongestionControl.scala` | 30/34 → **32/34** | 70, 71 |
| `ReplayLogManager.scala` | 30/34 → **31/34** | 122 |

**+7 fully-covered lines** (120/137 → 127/137 across the bundle). The
plain line-hit metric moves only +3, because four of the seven were
already executed and flip by completing a branch arm.

New files: `AsyncReplayLogWriterSpec` (7 tests),
`ReplayLogManagerImplSpec` (6 tests), and a shared
`ReplayLogWriterFixtures` support object. `AsyncReplayLogWriter` and
`ReplayLogManagerImpl` had no spec of their own before this.

The lines closed are the terminated-writer arms of `assert(!stopped)` in
`putLogRecords` and `putOutput`, the non-default `logInterval > 0` sleep
arm, the `ssThreshold` clamp reached only after five consecutive ack
timeouts, and `markAsReplayDestination`.

### Honesty about the bound

The measurement filter was 110 suite FQCNs — every spec under
`amber/src/test/scala/.../engine/**` plus `clustering/**`. CI's
full-module `WorkflowExecutionService/jacoco` runs 188. **Adding suites
can only raise the before-figure, so +7 is an upper bound on the CI
gain, not a floor.**

The specific lines closed are not plausibly reached by a `core/`
operator, tuple or storage spec, so I believe +7 is also the actual
figure — but a bound is what was measured and that is what is claimed.

### Four files from the original scope contribute nothing

`AmberFIFOChannel`, `WorkflowMessage`, `RecoveryPayload` and
`RegionPlan` were in the bundle and are worth **zero**; they already
have thorough specs, including `AmberMessageEnvelopesSpec`, whose name
mirrors neither source class it covers.

**`NetworkOutputGateway`'s tests were written and then deleted.** It
measured 22/25 before and after — lines 50, 51, 98 and 99 are already
covered by pre-existing suites at this scope. A third jacoco run on the
trimmed tree reproduces 127/137 exactly, confirming those tests
contributed nothing. `NetworkOutputGatewaySpec.scala` is byte-identical
to `HEAD` in this PR.

### Two hazards handled rather than discovered late

**`terminate()` can hang the build.** It does `stopped = true;
writerQueue.put(TerminateSignal); gracefullyStopped.get()` — an untimed
`CompletableFuture.get()` completed only at the end of `run()`. On a
writer that was never started it blocks forever, and amber's suites are
strictly serial in one JVM, so that hangs the module rather than failing
a test. Every test starts before terminating, with `terminate()` in a
`finally`; the class extends `Thread`, so a missed one would also leak a
live thread into the shared JVM.

**`logInterval` is a JVM-global.** It comes from
`ApplicationConfig.faultToleranceLogFlushIntervalInMs`, a memoized `val`
that is 0 by default. Poking that object would poison every later suite.
Instead the tests reflect on the **instance** field — `javap` confirms
`private final long logInterval`, and setting it via `Field.setLong` on
the instance was verified to work on this JDK. Instance-local, poisons
nothing.

### Deliberately not included

`AmberFIFOChannel` lines 47/48/51 are `logger.debug` bodies. There is no
`logback-test.xml`, ROOT governs, and CI sets
`TEXERA_SERVICE_LOG_LEVEL=WARN`, so the scala-logging `isDebugEnabled`
arm never fires in CI — they would look green locally and be missed in
CI. Not counted, and not chased by raising the log level in a test,
which would mutate a shared logging context in a serial JVM.

`NetworkOutputGateway:41` is the `AmberLogging` lazy `logger` accessor,
and the class body contains no logging call at all, so there is no call
site to reach it from. Covering it needs a production seam — refused.

### Verification

All mutations in the final table die, including the four the reviewers
measured as surviving the first draft (complete-before-close,
flush-after-release, drop-`Left`s, and the `INIT_STEP` mutant),
re-verified on the final tree.

Four `NetworkOutputGateway` mutants are recorded as killed by
**pre-existing** suites without help from this bundle, rather than
credited to it.

Measured with one fresh sbt JVM per side, the jacoco directory removed
between runs, an identical throwaway suite-name filter both times
(deleted afterwards), and `STORAGE_ICEBERG_CATALOG_TYPE=postgres` with
per-worktree namespaces — which takes the local baseline from 28
failures and 1 abort to **zero**, so neither run is a partial or
all-zero report.

No production file is touched.

### Any related issues, documentation, discussions?

Closes #8175

### How was this PR tested?

```
sbt "WorkflowExecutionService/testOnly 
org.apache.texera.amber.engine.architecture.logreplay.AsyncReplayLogWriterSpec 
org.apache.texera.amber.engine.architecture.logreplay.ReplayLogManagerImplSpec 
org.apache.texera.amber.engine.architecture.messaginglayer.CongestionControlSpec"
```

```
[info] Total number of tests run: 33
[info] Tests: succeeded 33, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```

`WorkflowExecutionService/Test/scalafmtCheck` and
`WorkflowExecutionService/Test/scalafix --check` both pass. Re-run after
rebasing onto current `main`.

### 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/33432353916

With regards,
GitHub Actions via GitBox

Reply via email to