aglinxinyuan opened a new issue, #7542: URL: https://github.com/apache/texera/issues/7542
### Task Summary `amber/src/main/scala/org/apache/texera/web/service/ExecutionStatsService.scala` has **no spec and sits at 0% of its 80 lines**, despite being the class that tells the frontend what an execution is doing: per-operator input/output metrics, worker assignment, elapsed duration, and the runtime statistics persisted for the dashboard's time series. Nothing about it needs infrastructure beyond what unit tests here already use. The seam is the three-argument constructor: ```scala new ExecutionStatsService(client: AmberClient, stateStore: ExecutionStateStore, workflowContext: WorkflowContext) ``` - `client` — an `AmberClient` subclass with an empty plan that captures the registered callbacks, so a test can fire `ExecutionStatsUpdate` / `RuntimeStatisticsPersist` / `WorkerAssignmentUpdate` / `WorkflowRecoveryStatus` / `FatalError` / `ExecutionStateUpdate` directly. This is the pattern `ExecutionRuntimeServiceSpec` and `ExecutionConsoleServiceSpec` already establish. - `stateStore` — a real `ExecutionStateStore`, so the constructor-registered diff handlers run and can be observed through `statsStore.getWebsocketEventObservable`. - `workflowContext` — selects the runtime-statistics storage URI, and is therefore the isolation knob. Three things to know before starting: 1. **URI collision.** The runtime-statistics URI derives from `workflowId`/`executionId`, and `DocumentFactory.createDocument` passes `overrideIfExists = true`. A default `new WorkflowContext()` yields `vfs:///wid/1/eid/1/runtimestatistics` — the same URI `DefaultCostEstimatorSpec` uses. sbt runs amber suites in parallel in one JVM, so the spec must use distinct ids. 2. **Catalog.** The committed default is `storage.iceberg.catalog.type = rest`, which expects a Lakekeeper. CI's unit job sets `STORAGE_ICEBERG_CATALOG_TYPE=postgres` (`.github/workflows/build.yml:293`); running the spec locally needs the same env var. 3. **Do not install a temp catalog.** `IcebergCatalogInstance.replaceInstance` is JVM-wide and the URI carries no warehouse, so it would hijack the catalog for every other amber suite in the same JVM. Two lines are unreachable and should not be tested: the `catch` around `runtimeStatsWriter.close()` (Iceberg's close is idempotent), and the `catch` in `storeRuntimeStatistics` (it runs on a private single-thread executor, which swallows the throwable, so no assertion could observe the difference). Separately, `computeStatsDiff` contains three provably dead lines — `defaultMetrics`, `newKeys`, and the `++ newKeys.map(_ -> defaultMetrics)` merge. `updatedLastMetrics` is read only at `oldKeys.map(key => key -> updatedLastMetrics(key))`, and `oldKeys` and `newKeys` are disjoint by construction, so the merged-in entries can never be selected. Worth deleting rather than testing. ### 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]
