fix(test): stop the file-handle leak aborting the operator suite, seed the interval spec

[aglinxinyuan]

What changes were proposed in this PR?

Three test-only fixes. 45 insertions, 9 deletions across 3 spec files; git diff -- '*/src/main/*' is empty.

1. A leaked file handle was aborting the operator suite and killing the module's coverage run

The exact mechanism, established by instrumenting the run rather than by inspection: in "create LargeBinary when reading file with LARGE_BINARY type", createTuplesFromFile throws mid-map with IllegalStateException: LargeBinaryManager.create() requires a base URI, but none was set on the current thread. That abandons the AutoClosingIterator before exhaustion, so its close-on-exhaustion hook never fires. afterAll then fails with test_large_binary.txt: The process cannot access the file because it is being used by another process, aborting the suite.

The blast radius is the whole module:

	before	after
Suites	289 completed, 1 aborted	290 completed, 0 aborted
sbt exit	1	0
jacoco report	no report directory emitted	jacoco.xml, 1.8 MB
Tests	2301 succeeded	2301 succeeded

So WorkflowOperator/jacoco could not produce coverage for any file in the module on Windows. POSIX unlink masks this on Linux CI.

The fix is in the test: the read is wrapped so the iterator is drained in a finally, firing the close hook on both the success and failure paths. AutoClosingIterator and createTuplesFromFile are untouched — the leak is a test that stops early, not a broken production contract.

Also worth recording: the .zip fixture was stranded too. It only looked fine because .gitignore:10 is *.zip; afterAll never reached it, because the .txt delete threw first.

2. IntervalOpExecSpec is now deterministic

It imported scala.util.Random.{nextInt, nextLong} and used them at four sites — input ordering twice, a 1k-row dataset, and the interval constant — so IntervalJoinOpExec's coverage footprint drifted between runs of identical source (CI has reported 2 missed + 14 partial where a local run gave 0 + 13).

Now a fixed Seed, with a fresh new Random(Seed) created at each use site so determinism does not depend on test execution order either. No assertion changed.

Verified rather than assumed: instrumented to print every generated input, two runs produced byte-identical output (22 lines, empty diff), and two full WorkflowOperator/jacoco runs now give byte-identical IntervalJoinOpExec counters and all 110 per-line entries.

One observation left alone as out of scope: the 1k test deterministically yields 0 matches, because random 64-bit longs essentially never fall inside a sub-1000-wide window, so outputTuples.size == bruteForceResult is 0 == 0. That was equally true before this change.

3. PveResourceSpec's traversal assertion said something it did not test

getPythonBin(testCuid, "..") shouldBe None appeared to pin the guard at PveManager.scala:91, but ".." matches the name regex on line 88 (^[A-Za-z0-9._-]+$ admits dots), and <VenvRoot>/<cuid>/../pve normalizes to <VenvRoot>/pve — still under root. The None came from the Files.exists check on line 92.

Proven, not argued: deleting line 91 produced results identical to baseline (43 run, 37 succeeded, 6 failed, 1 canceled) and the traversal test still passed. Nothing in the spec detects that guard's removal. The production file was reverted afterwards.

Split into two honest cases:

• "reject pveNames containing a path separator" keeps "../../../etc" and "foo/bar" — both carry a /, so they genuinely pin the name regex.
• "return None for a dot-only pveName, which has no venv" keeps the ".." case and pins what it actually pins: a name with no venv on disk yields None.

A comment records that line 91 is unreachable by construction — the regex forbids / and cuid is an Int — so it is defensive code rather than untested code. The guard is left in place; removing it is a production decision, not a test one.

Verification

• Full WorkflowOperator/jacoco: 290 suites, 0 aborted, 2301 tests succeeded, sbt exit 0, report emitted.
• PveResourceSpec: 44 run, 38 succeeded — both reshaped tests pass. The 6 failures are pre-existing and environmental (the mock fabricates bin/python while PveManager looks for Scripts/python.exe on Windows); baseline shows the identical 6.
• git diff -- '*/src/main/*' is empty; no stranded fixtures (checked with git status --ignored).
• Lint green: WorkflowOperator/Test/scalafmtCheck, WorkflowOperator/scalafixAll --check, WorkflowExecutionService/Test/scalafmtCheck, WorkflowExecutionService/scalafixAll --check.

One further defect found, deliberately not fixed here

The same FileScanSourceOpExecSpec test wraps its assertions in catch { case e: Exception => info(...) }. ScalaTest's TestFailedException extends Exception, and info output is invisible under this build's -u-only reporter (build.sbt:37) — so that test currently cannot fail. Narrowing the catch (or using cancel/assume) would make it a real test again, but it needs care about how LargeBinaryManagerSpec binds a base URI, so it does not belong in this change.

Any related issues, documentation, discussions?

Closes #7799

How was this PR tested?

sbt "WorkflowOperator/jacoco"

[info] Suites: completed 290, aborted 0
[info] Tests: succeeded 2301, failed 0, canceled 0, ignored 0, pending 2

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 5)

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Backport auto-label report

This fix: PR was checked against each actively-supported release branch. release/* labels drive the post-merge backport, so add or remove one to change where this fix lands.

Release branch	Analysis
✅ release/v1.2	Change detected on this branch — label added; this fix is queued to backport here. Requested review from @xuang7.

Auto-label run.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @mengw15
  You can notify them by mentioning @mengw15 in a comment.

[github-actions]

⚠️ Benchmark changes need a look

🟢 4 better · 🔴 3 worse · ⚪ 8 noise (<±5%) · 0 without baseline

Compared against main 88426b3 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	379	0.231	25,815/32,503/32,503 us	🟢 -14.8% / 🔴 +108.4%
🟢	bs=100 sw=10 sl=64	797	0.487	124,342/136,186/136,186 us	🟢 -20.9% / 🔴 +27.5%
⚪	bs=1000 sw=10 sl=64	909	0.555	1,099,853/1,161,647/1,161,647 us	⚪ within ±5% / 🔴 +14.6%

Baseline details

Latest main 88426b3 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	379 tuples/sec	405 tuples/sec	771.02 tuples/sec	-6.4%	-50.8%
bs=10 sw=10 sl=64	MB/s	0.231 MB/s	0.247 MB/s	0.471 MB/s	-6.5%	-50.9%
bs=10 sw=10 sl=64	p50	25,815 us	22,873 us	12,749 us	+12.9%	+102.5%
bs=10 sw=10 sl=64	p95	32,503 us	38,149 us	15,594 us	-14.8%	+108.4%
bs=10 sw=10 sl=64	p99	32,503 us	38,149 us	19,320 us	-14.8%	+68.2%
bs=100 sw=10 sl=64	throughput	797 tuples/sec	791 tuples/sec	997.05 tuples/sec	+0.8%	-20.1%
bs=100 sw=10 sl=64	MB/s	0.487 MB/s	0.483 MB/s	0.609 MB/s	+0.8%	-20.0%
bs=100 sw=10 sl=64	p50	124,342 us	119,359 us	100,339 us	+4.2%	+23.9%
bs=100 sw=10 sl=64	p95	136,186 us	172,235 us	106,781 us	-20.9%	+27.5%
bs=100 sw=10 sl=64	p99	136,186 us	172,235 us	113,101 us	-20.9%	+20.4%
bs=1000 sw=10 sl=64	throughput	909 tuples/sec	921 tuples/sec	1,037 tuples/sec	-1.3%	-12.4%
bs=1000 sw=10 sl=64	MB/s	0.555 MB/s	0.562 MB/s	0.633 MB/s	-1.2%	-12.3%
bs=1000 sw=10 sl=64	p50	1,099,853 us	1,089,203 us	971,388 us	+1.0%	+13.2%
bs=1000 sw=10 sl=64	p95	1,161,647 us	1,149,073 us	1,013,249 us	+1.1%	+14.6%
bs=1000 sw=10 sl=64	p99	1,161,647 us	1,149,073 us	1,039,879 us	+1.1%	+11.7%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,528.05,200,128000,379,0.231,25815.17,32502.58,32502.58
1,100,10,64,20,2508.30,2000,1280000,797,0.487,124341.59,136186.08,136186.08
2,1000,10,64,20,21996.18,20000,12800000,909,0.555,1099852.76,1161647.30,1161647.30

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.22%. Comparing base (c927890) to head (301c6d5).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7800      +/-   ##
============================================
- Coverage     91.25%   91.22%   -0.03%     
- Complexity     4471     4475       +4     
============================================
  Files          1171     1171              
  Lines         47124    47170      +46     
  Branches       5286     5305      +19     
============================================
+ Hits          43003    43033      +30     
- Misses         2447     2462      +15     
- Partials       1674     1675       +1     

Flag	Coverage Δ		*Carryforward flag
access-control-service	81.00% <ø> (ø)
agent-service	98.62% <ø> (ø)		Carriedforward from c927890
amber	88.03% <ø> (+0.01%)	⬆️
computing-unit-managing-service	73.67% <ø> (ø)
config-service	86.73% <ø> (ø)
file-service	68.90% <ø> (ø)
frontend	92.63% <ø> (ø)		Carriedforward from c927890
notebook-migration-service	79.11% <ø> (-4.63%)	⬇️
pyamber	97.57% <ø> (ø)		Carriedforward from c927890
workflow-compiling-service	77.19% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.