aglinxinyuan opened a new pull request, #7774: URL: https://github.com/apache/texera/pull/7774
### What changes were proposed in this PR? Four small files across `common/workflow-core` and `common/workflow-operator`, each with one narrow untested region. 14 tests added (65 -> 79 across the four specs). Measured with `jacoco` under a `Test/testOptions` filter, the same filter for the before and after runs: | File | Lines before | Lines after | Branches | |---|---|---|---| | `ExecFactory.scala` | 17/19 (89.5%) | **19/19 (100%)** | 4/4 | | `AggregateOpExec.scala` | 14/16 (87.5%) | **16/16 (100%)** | 2/2 | | `FileScanUtils.scala` | 39/50 (78.0%) | **50/50 (100%)** | 12/20 -> **19/20** | | `OPVersion.java` | 9/17 (52.9%) | **14/17 (82.4%)** | 2/2 | One number needs a caveat rather than a headline: the `FileScanUtils` **78.0%** baseline is measured with only `FileScanUtilsSpec` running. The reported module-wide figure is 90.0%, because `FileScanSourceOpExecSpec` also touches that file — it is excluded here for the reason below, from both runs, so the before/after comparison is apples-to-apples even though the baseline differs from Codecov's. Beyond the four target regions, `FileScanUtils`' 7-argument `createTuplesFromFile` overload turned out to be uncovered as well and is now covered, which accounts for most of that file's jump. ### Two things that look untestable and are not **`newExecFromJavaCode` can be driven for real, and a stale comment said otherwise.** `CoreExecutorReflectionSpec` claimed a success-path test was impossible because system javac would see only its own classpath. `common/workflow-core/build.sbt` sets `Test / fork := true`, so the forked JVM's `java.class.path` is the full test classpath and `compileCode`'s null options let javac resolve `OperatorExecutor`. A Java class implementing the trait now compiles, instantiates and dispatches through it. The comment is replaced with the mechanism. **`OPVersion.getVersion` needs no real repository history** — throwaway jgit repos in temp dirs are swapped into the private static `git` field by reflection and restored in a `finally`, following `JGitVersionControlSpec`. ### Verification 13 mutations, **12 killed, 1 equivalent**. Anchor uniqueness was asserted for all 13 before any edit; each was applied alone, reverted, with `git diff -- '*/src/main/*'` confirmed empty after every revert. Every run executed a full suite (26/26/18/9 tests, never 0), so no result is a compile-only error. | Mutation | Killed by | |---|---| | `ExecFactory`: **exchange** sibling factories, body -> `newExecFromJavaClassName(code)` | compiles java source into a live OperatorExecutor (+2) | | `ExecFactory`: swallow a failed compile, return `null` | surfaces the compiler diagnostics when the java source does not compile | | `ExecFactory`: memoize the first executor and return it forever | hands back an independent instance on every call (+1) | | `AggregateOpExec`: remove `keyedPartialAggregates.clear()` | `close()` discards the accumulated groups | | `AggregateOpExec`: `close()` becomes a no-op | same test | | `FileScanUtils`: **exchange** the BINARY and SINGLE_STRING hint literals | translates an out-of-memory read into advice to switch to large binary (+1) | | `FileScanUtils`: **exchange** the `case _` hint with the BINARY hint | falls back to a generic hint for any other attribute type | | `FileScanUtils`: flip ONE catch leg, `IllegalArgumentException` -> `IOException` | treats an IllegalArgumentException as an over-size failure too (+1) | | `FileScanUtils`: **exchange** `fileScanOffset` and `fileScanLimit` in the overload | forwards the offset and limit through the seven-argument overload | | `FileScanUtils`: **exchange** `displayFileName = fileName` -> `attributeType.getName` | reads a file through the seven-argument overload under its own name | | `OPVersion`: flip the memo guard `!containsKey` -> `containsKey` | 8 of 9 tests | | `OPVersion`: **exchange** the two in-scope strings in `put(...)` | resolves the newest commit that touched the operator's own path (+1) | **The equivalent mutant:** removing `distributedAggregations = null` from `close()` survives, and that is correct. `close()` is terminal; `onFinish` reads the field once per accumulated group and line 42 leaves none; a reused executor's `open()` re-nulls it anyway. The reset is observable only via `processTuple` after `close()` with no intervening `open()` — an order the executor lifecycle never produces. Recorded in the spec as deliberately not asserted, rather than cemented with an illegal-sequence test. ### Deliberately not included `FileScanUtils` line 99 stays branch-partial: an unreachable `Tuple2` `MatchError` leg from `Iterator.duplicate`. `OPVersion` lines 31/36/39 are the static initializer, which runs before any test can observe it — and which pair is hit depends on checkout shape, since a `git worktree` makes `.git` a file. Three observations are reported rather than pinned: - **`OPVersion.getVersion` returns `null` on `GitAPIException`.** That catch calls `printStackTrace()` and never populates `opMap`, so the trailing `opMap.get(operatorName)` hands back `null` and every later call retries the failing `git log`. Its `NullPointerException` sibling stores `"N/A"`. The test asserts only the swallow, not the `null`, so a fix is not blocked. - **`FileScanSourceOpExecSpec` leaks a file handle on Windows.** Its `afterAll` fails with `FileSystemException … test_large_binary.txt: The process cannot access the file because it is being used by another process`, which aborts the suite and takes the whole `WorkflowOperator/jacoco` task with it. `AutoClosingIterator` closes only on exhaustion, so a test stopping early leaks the handle; POSIX `unlink` masks it on CI. Pre-existing, and the reason that spec is filtered out of the measurements above. - **`AggregateOpExec.close()` NPEs if `open()` was never called** (`keyedPartialAggregates` is null). Lifecycle-guarded in practice. No production file is touched. ### Any related issues, documentation, discussions? Closes #7773 ### How was this PR tested? ``` sbt "WorkflowCore/testOnly org.apache.texera.amber.core.executor.CoreExecutorReflectionSpec" "WorkflowOperator/testOnly org.apache.texera.amber.operator.aggregate.AggregateOpSpec org.apache.texera.amber.operator.source.scan.file.FileScanUtilsSpec org.apache.texera.amber.operator.metadata.OPVersionSpec" ``` ``` [info] Tests: succeeded 26, failed 0, canceled 0, ignored 0, pending 0 [info] Tests: succeeded 53, failed 0, canceled 0, ignored 0, pending 0 ``` 0 suites aborted. `WorkflowCore/Test/scalafmtCheck`, `WorkflowOperator/Test/scalafmtCheck` and both modules' `Test/scalafix --check` all 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]
