The GitHub Actions job "Required Checks" on texera.git/main has failed. Run started by GitHub user github-merge-queue[bot] (triggered by github-merge-queue[bot]).
Head commit for run: b7c33b0892db49a94a3b15c81eed4ebcbc12882d / Xinyuan Lin <[email protected]> test(workflow-core): cover the lakeFS file document against the loopback stub (#7831) ### What changes were proposed in this PR? Test-only. **No production file changes** — `git diff -- '*/src/main/*'` is empty. `LakeFSFileDocument` looked Docker-gated and is not: `LakeFSStorageClientSpec` (merged in #7273) already drives `LakeFSStorageClient` against an in-process `com.sun.net.httpserver` loopback stub, and already covers the exact two calls the document makes. This PR reuses that harness. Measured with `WorkflowCore/jacoco` under an identical filter for the before and after runs (the Docker-backed `LakeFSStorageClientMtimeSpec` is excluded from **both**, so these differ from the Codecov figures, which include it): | File | Lines | Branches | |---|---|---| | `LakeFSFileDocument.scala` | 32/80 (40.0%) → **65/80 (81.2%)** | 16/46 → **27/46** | | `LakeFSStorageClient.scala` | 120/146 (82.2%) → **133/146 (91.1%)** | 41/60 → **45/60** | **+33 lines and +11 branch arms** on the document, **+13 lines** on the client. Tests **38 → 49**. Covered: `asInputStream`'s presign happy path and its fallback (the local `def fallbackToLakeFS` lifts to `fallbackToLakeFS$1`, a counted method rather than an `$anonfun$`, which is why these lines move the number at all), `asFile` both arms including the `tempFile` memoization, `clear` both arms, and `LakeFSStorageClient.put`. The 15 lines still missed on the document are exactly the out-of-scope region — lines 133-161, the `userJwtToken`-non-empty file-service presign branch. Reaching it needs a forked JVM with `USER_JWT_TOKEN` set **and** a presign endpoint on a *fixed* port known before JVM start, i.e. a `build.sbt` `testGrouping` change plus a hardcodable port that can collide in CI. Deliberately excluded. ### Verification Review proposed 8 findings. **Six were real survivors and are now killed**, each proved red on a named test after the repair; two are genuinely unpinnable here and are recorded in the spec rather than papered over. Suite 47 → 49 after repair. | Survivor found by review | How it is now killed | |---|---| | both `asInputStream` call sites reduced to `fileRelativePath.getFileName.toString` | a nested-fixture read test that kills the pair *and* each site alone | | widening the `put` success guard to `code >= 400` | rejection test extended to 403/204/307; the 204 and 307 single-leg widenings each die separately | | deleting `inputStream.close()` | a recording-subclass test, deterministic on any platform | | re-downloading into the memoized `File` | `requests.clear()` plus `hits shouldBe empty` | | dropping `setChunkedStreamingMode` | the stub now records request headers; the put test pins `content-length: 5` and the absence of `transfer-encoding` | | `Files.exists shouldBe true` was JDK-guaranteed | now `Files.size == 2502` — and the mutant fails *at that line* rather than 12 lines later | **Two limits, argued rather than cargo-culted.** The fallback `logger.warn` at :110 cannot be pinned: `workflow-core`'s test classpath has no SLF4J provider at all ("No SLF4J providers were found", NOP loggers), and scala-logging's `isWarnEnabled` guard means the call is not even evaluated — a capturing appender has nothing to attach to. And `deleteRepo` (1 line) was left alone: it is `repoApi.deleteRepository(name).execute()` with no branch, so the only assertable fact is the generated SDK's own URL template. The existing spec already documents that rationale and I did not contradict it for one line. ### Two production bugs found while doing this, reported not fixed Both are out of scope for a test-only PR and neither is cemented by any new test. 1. **`LakeFSFileDocument.scala:204` renders the relative path with the platform separator.** On a Windows worker, every nested dataset/model file is requested as `dir\file.csv` in the lakeFS `path` query parameter — 404, silent fallback, 404 again. A reviewer's suggested assertion of `Some("nested/dir/records.csv")` would be red against today's production, which is how this surfaced. Fix is a one-line `'/'`-join and wants its own PR. 2. **`tempFileStream.close()` is load-bearing on Windows only.** Deleting it fails 4 tests here with "file is being used by another process" (the cleanup hits an open write handle) while shipping green through Linux CI. `FileOutputStream` is unbuffered, so no portable in-process assertion can observe it; wrapping the copy in `try`/`finally` is production work. ### Any related issues, documentation, discussions? Closes #7830 ### How was this PR tested? ``` sbt "WorkflowCore/testOnly org.apache.texera.amber.core.storage.model.LakeFSFileDocumentSpec org.apache.texera.amber.core.storage.util.LakeFSStorageClientSpec" ``` ``` [info] Suites: completed 2, aborted 0 [info] Tests: succeeded 49, failed 0, canceled 0, ignored 0, pending 0 ``` `scalafmtCheckAll` and `scalafixAll --check` both pass. `LakeFSFileDocumentSpec` gains `@NonParallelTest` so it gets its own forked JVM, matching the sibling spec. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Report URL: https://github.com/apache/texera/actions/runs/32569336610 With regards, GitHub Actions via GitBox
