Skip to content

test(dashmate): make ZeroSSL provider-change test deterministic - #4482

Merged
QuantumExplorer merged 1 commit into
v4.2-devfrom
test/dashmate-zerossl-flaky
Aug 26, 2026
Merged

test(dashmate): make ZeroSSL provider-change test deterministic#4482
QuantumExplorer merged 1 commit into
v4.2-devfrom
test/dashmate-zerossl-flaky

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Aug 26, 2026

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

The dashmate unit test "should detect a provider change before the old certificate renewal date" intermittently fails with Timeout of 10000ms exceeded on GitHub-hosted runners (example failure, on a PR with zero JS changes).

The test ends with a blanket clock.tickAsync(64 days). That tick is free when the provider handoff cleared every timer, but whenever the 60-second config-refresh timer survives into it, sinon must replay ~92k firings — each one a real event-loop hop plus stub call bookkeeping — which blows the 10s mocha timeout on a slow shared runner instead of failing on an assertion. The CI log shows exactly this shape: 10.00s of silence between the test's scheduling log line and the timeout, while the structurally identical sibling test passed in 6ms right after.

What was done?

  • Replaced the await clock.tickAsync(64 * 24 * 60 * 60 * 1000) in packages/dashmate/test/unit/helper/scheduleRenewZeroSslCertificateFactory.spec.js with await clock.runAllAsync(), with a comment explaining why.
  • This preserves — and strengthens — what the test proves: instead of "no ZeroSSL obtain task runs within 64 days of the provider switch", it now proves the handoff leaves nothing armed at all, so the old renewal can never fire. It is the only long tick in the file; sibling tests tick single 60-second refresh periods, which are unaffected.
  • Failure modes are now fast and attributable: a cron job surviving the handoff fires during runAllAsync and trips the calledOnce/not.called assertions (~50ms); a surviving refresh interval trips fake-timers' 1000-timer infinite-loop guard (~170ms). Both verified by temporarily seeding those regressions into scheduleRenewalJob.js/watchCertificateConfig.js.

How Has This Been Tested?

  • yarn workspace dashmate mocha test/unit/helper/scheduleRenewZeroSslCertificateFactory.spec.js run 18× (mixed TZ=UTC and local timezone to match CI): 16 passing every run, 42–61ms per run.
  • Mutation checks: with job.stop() removed from the config-change handoff the test fails on the assertion in ~50ms; with clearInterval removed from the watcher it fails with sinon's infinite-loop guard in ~170ms — instead of a 10s timeout in both cases.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved certificate renewal scheduling behavior when switching providers.
    • Confirmed that outdated ZeroSSL renewal timers do not run after the provider handoff.

The provider-change test ticked a blanket 64 days after the handoff. That
is free when the handoff cleared every timer, but whenever a 60-second
config-refresh timer survives into the tick, tickAsync must replay ~92k
firings — each one a real event-loop hop plus sinon call bookkeeping —
which blows the 10s mocha timeout on a slow shared runner instead of
failing on an assertion (e.g. run 32935417801: 10.00s of silence between
the scheduling log line and the timeout, while the structurally identical
sibling test passed in 6ms right after).

Replace the 64-day tick with clock.runAllAsync(), which proves a stronger
statement: the handoff leaves nothing armed at all, so the old renewal can
never fire. Verified against seeded regressions: a cron job surviving the
handoff now fails on the calledOnce assertion in ~50ms, and a surviving
refresh interval fails with fake-timers' infinite-loop guard in ~170ms —
both instead of a 10-second timeout.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v4.2.0 milestone Aug 26, 2026
@thepastaclaw

thepastaclaw commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

🕓 Ready for review — next in queue (commit c32e3b5)
Queue position: 1/1
ETA: start ~07:30 UTC · complete ~07:52 UTC (median 22m across 30 recent reviews; 2 slots)
Queued 2m ago · Last checked: 2026-08-26 07:30 UTC

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a534a672-1d5d-47e2-b247-ddfab28047a4

📥 Commits

Reviewing files that changed from the base of the PR and between d03cc04 and c32e3b5.

📒 Files selected for processing (1)
  • packages/dashmate/test/unit/helper/scheduleRenewZeroSslCertificateFactory.spec.js

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The provider-change test now drains pending timers with clock.runAllAsync() and verifies that no ZeroSSL renewal timer fires after handoff.

Changes

ZeroSSL renewal timer validation

Layer / File(s) Summary
Drain timers after provider handoff
packages/dashmate/test/unit/helper/scheduleRenewZeroSslCertificateFactory.spec.js
The test replaces a 64-day fake-time advance with clock.runAllAsync() and retains the assertion that ZeroSSL renewal remains inactive.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to c32e3

This localized test-only change makes the ZeroSSL provider-change test deterministic without changing production behavior; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: shumkov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: making the Dashmate ZeroSSL provider-change test deterministic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/dashmate-zerossl-flaky

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@QuantumExplorer

Copy link
Copy Markdown
Member Author

Reviewed

@QuantumExplorer
QuantumExplorer merged commit 7f6d429 into v4.2-dev Aug 26, 2026
16 checks passed
@QuantumExplorer
QuantumExplorer deleted the test/dashmate-zerossl-flaky branch August 26, 2026 07:32
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.

2 participants