Skip to content

Two run tests assumed timings a hosted runner does not give (0.6.8) - #60

Merged
sotashimozono merged 1 commit into
mainfrom
timing-tests
Sep 29, 2026
Merged

sotashimozono merged 1 commit into
mainfrom
timing-tests

Conversation

@sotashimozono

Copy link
Copy Markdown
Member

DataVault.jl's downstream SweepRunner job failed 3 times out of 15 identical CompatHelper PRs (#60, #62, #72). Each failure was one of these two assertions, and the contents of those PRs were identical to the 12 that passed. The EOFError / Error encountered while load … manifest.jld2 lines in those logs appear in passing runs too, the same number of times, and are not the failure.

test_run_loop_busy.jl:51 — elapsed >= 3.0 (failed at 2.85 s and 2.90 s)

DataVault writes heartbeat= truncated to the second ("yyyy-mm-ddTHH:MM:SS"), so a lock reads up to 1 s older than it is and is reclaimed up to 1 s before stale_after.

Measured locally:

lock written at age right after writing age > 3.0 first seen at
.05 s 0.06 s 2.96 s
.95 s 0.94 s 2.07 s, 2.08 s

When elapsed lands between 2 and 3 s depends on where the round boundaries fall, which is why the test failed only sometimes. The bound is now stale_after − 1.

Does it still catch the bug it pins? With the busy wait disabled (if false && result.busy > 0), run_loop! returned in 0.96 / 0.99 / 1.01 s with done == 0 after warm-up, so >= 2.0 fails as it should. r.done == 1 and is_done also catch it. The first cold run took 9.7 s because of compilation and reclaimed the key; the old >= 3.0 misses that case too, so the new bound is no weaker.

With stale_after = 600 in production, reclaiming up to 1 s early does not matter. The .running format is a contract, so DataVault is not changed.

test_run_deadline.jl:58 — started[] >= 1 (was 0)

A deadline 0.3 s ahead passed before run! handed out its first key: a run-start observation takes most of a second. This is the same cause as e207f53. RunOpts is immutable and deadline is absolute, so the fix cannot trigger on the first key the way that commit did. Instead the deadline is 5 s ahead and the first key runs until it has passed. The test still checks the same thing (at least one key started, not every key, returned after the deadline, stopped_by === :deadline) and no longer depends on how fast run! starts. It takes about 5 s longer.

Checked

  • Both files pass locally in a throwaway env (test_run_loop_busy.jl 23 s, test_run_deadline.jl 19 s). JuliaFormatter 2 reports them already formatted.

🤖 Generated with Claude Code

DataVault's downstream job failed 3 times in 15 identical runs, each on
one of these assertions:

- test_run_loop_busy.jl: `elapsed >= 3.0` (stale_after) failed at 2.85 s
  and 2.90 s. DataVault writes `heartbeat=` truncated to the second, so
  a lock reads up to 1 s older than it is and is reclaimed up to 1 s
  before stale_after. Measured: a lock written at .95 s is reclaimable
  2.07 s later. Now `>= stale_after - 1`. With the busy wait disabled
  (the bug this pins), run_loop! returns in 0.96-1.01 s with done == 0,
  so the new bound still catches it.
- test_run_deadline.jl: a deadline 0.3 s ahead passed before `run!`
  handed out its first key (started == 0); a run-start observation
  takes most of a second. The deadline is now 5 s ahead and the first
  key runs until it has passed, so the test no longer depends on how
  fast `run!` starts. Same cause as e207f53.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📚 Docs preview: https://qatlashub.github.io/SweepRunner.jl/previews/PR60/

(updates on each push to this PR)

@codecov

codecov Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sotashimozono
sotashimozono merged commit 509eb74 into main Sep 29, 2026
17 checks passed
@sotashimozono
sotashimozono deleted the timing-tests branch September 29, 2026 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant