Yicong-Huang opened a new pull request, #7051:
URL: https://github.com/apache/texera/pull/7051

   ### What changes were proposed in this PR?
   
   De-flake `S3StorageClientSpec`, which intermittently fails **4 tests** in 
the `build / amber` CI job (scope `WorkflowCore / Test / test`).
   
   **Root cause.** Four suites mix `S3StorageTestBase` and share a JVM-wide 
`S3StorageClient.s3Client` plus a single `StorageConfig.s3Endpoint`, all 
pointed at one shared MinIO container: `S3StorageClientSpec`, 
`LargeBinaryManagerSpec`, `LargeBinaryInputStreamSpec`, 
`LargeBinaryOutputStreamSpec`. ScalaTest's parallel suite distributor runs them 
concurrently within a single in-process test task, so they contend for that one 
container and intermittently time out. The `Global / concurrentRestrictions += 
Tags.limit(Tags.Test, 1)` in `common/workflow-core/build.sbt` only limits sbt 
**task** concurrency; it does not stop ScalaTest from parallelizing suites 
inside one unforked test task (CI logs show 
`pool-N-thread-M-ScalaTest-running-...Spec`).
   
   **Fix.**
   - `common/workflow-core/build.sbt`: give each MinIO-backed suite its own 
forked JVM via `Test / testGrouping`, exactly mirroring the existing 
FileService approach for testcontainer suites (`build.sbt`, `FileService`). sbt 
runs forked groups one at a time (`Tags.ForkedTestGroup` limit), so these 
suites never overlap and each gets a clean MinIO container; every other 
workflow-core suite stays in one in-process group and keeps running in 
parallel. The forked working directory is pinned to the repo root to match the 
in-process cwd.
   - `S3StorageClientSpec.scala`: bound the concurrent-upload burst in the 
*>1000 objects* test from 16 to 4 threads, so a single suite no longer floods 
the container.
   
   **Why not tag these `@IntegrationTest`?** That tag/lane lives only in the 
`amber` module (`amber/src/test/integration/.../tags/IntegrationTest.java`). 
`amber` depends on `workflow-core`, so a workflow-core suite cannot reference 
it. The established convention for common-module container-backed tests (e.g. 
file-service's LakeFS/MinIO suites) is to keep them in normal `Test` scope and 
isolate them via forked-per-suite JVMs — which is what this PR does.
   
   ### Any related issues, documentation, discussions?
   
   Closes #7049
   
   ### How was this PR tested?
   
   - `sbt reload` loads the build cleanly (settings resolve, no new lint 
warnings from the change).
   - The change mirrors the proven FileService test-grouping pattern; the 
runtime serialization is validated by CI (the MinIO suites require Docker, 
which is unavailable in the authoring environment). Since the failure is flaky, 
reviewers may want to re-run the `amber` job a couple of times to confirm 
stability.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 4.8 [1M context])
   


-- 
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]

Reply via email to