fix(test): fix CI hang, auth guard tests, and PR #610 test rewrite#616
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛Upgrade
Other
Documentation 📚
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // this, re-initializing the SDK (e.g., in tests) leaks setInterval handles | ||
| // that keep the event loop alive and prevent the process from exiting. | ||
| // close(0) removes listeners synchronously; we don't need to await the flush. | ||
| Sentry.getClient()?.close(0); |
There was a problem hiding this comment.
Global client closed during reinitialization
Medium Severity
initSentry now unconditionally calls Sentry.getClient()?.close(0) before every init. In libraryMode, concurrent SDK invocations can close another in-flight invocation’s client, which can drop telemetry and invalidate active spans. It can also close a pre-existing process-wide Sentry client not created by this module.
| listener as (...args: unknown[]) => void | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Overbroad beforeExit listener cleanup
Low Severity
afterAll removes every beforeExit listener not present at file load, not just listeners created by this test. In shared test workers, this can unregister unrelated listeners added by other modules, causing cross-test interference and hard-to-reproduce failures.
betegon
force-pushed
the
fix/ci-hang-and-auth-guard-tests
branch
from
f022b2a to
d14e1cd
Compare
Codecov Results 📊✅ 129 passed | Total: 129 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1349 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 95.73% 95.46% -0.27%
==========================================
Files 204 204 —
Lines 29877 29738 -139
Branches 0 0 —
==========================================
+ Hits 28601 28389 -212
- Misses 1276 1349 +73
- Partials 0 0 —Generated by Codecov Action |
Three independent issues caused CI to hang and ~200 tests to fail: 1. **CI hang (telemetry.ts)**: `beforeExit` handler removal was inside `if (client?.getOptions().enabled)`, so `initSentry(false)` in test afterEach hooks never cleaned up the handler from a prior `initSentry(true)`. Additionally, SDK-internal handlers from `enableLogs` and `sendClientReports` were never cleaned up on re-init. Fix: close previous client before re-init, move handler removal outside enabled guard, gate enableLogs/sendClientReports on `enabled`. 2. **Auth guard test failures (PR #611)**: `buildCommand` now requires auth by default. Fix: set fake SENTRY_AUTH_TOKEN in test preload, add `auth: false` to command.test.ts framework tests, save/clear/ restore env token in auth-specific tests. 3. **Dead code tests (PR #610)**: `createSentryProject` no longer does org resolution (moved to wizard-runner's `resolvePreSpinnerOptions`). Fix: rewrite 15 tests to call `resolveOrgSlug` and `detectExistingProject` directly instead of through `handleLocalOp`. Also fixes mtime test flakiness by replacing `Bun.sleep(10)` with explicit `utimes()` calls in dsn-cache and project-root-cache tests. Made-with: Cursor
betegon
force-pushed
the
fix/ci-hang-and-auth-guard-tests
branch
from
d14e1cd to
8c2dc2f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| "my-org/my-project" | ||
| ) | ||
| ).rejects.not.toThrow(ValidationError); | ||
| resolveOrgProjectSpy.mockRestore(); |
There was a problem hiding this comment.
Spy cleanup skipped on assertion failure
Low Severity
The test creates resolveOrgProjectSpy and restores it only at the end of the happy path. If an assertion throws before mockRestore(), the mock remains active and can affect later tests in test/commands/log/list.test.ts, causing cascading failures and misleading CI results.
1 participant
Summary
Fixes the CI unit test hang and ~200 test failures caused by three merged PRs (#610, #611). Supersedes #612 with a cleaner approach.
1. CI Hang — telemetry
beforeExithandler leakinitSentry(false)in testafterEachhooks never cleaned up thebeforeExithandler registered by a priorinitSentry(true)call — the removal was gated onclient?.getOptions().enabled. SDK-internal handlers fromenableLogsandsendClientReportsalso accumulated on re-init.Fix (src/lib/telemetry.ts):
Sentry.getClient()?.close(0))enabledguardenableLogsandsendClientReportsonenabled(not justlibraryMode)ProcessSessionbeforeExitlisteners in telemetry.test.ts2. Auth guard test failures (PR #611)
buildCommandnow callsgetAuthConfig()before every command. Tests had no auth token.Fix:
SENTRY_AUTH_TOKENin test/preload.ts (blocked by global fetch mock)auth: falseto all 34buildCommandcalls in command.test.tsSENTRY_AUTH_TOKENin 9 test files that verify unauthenticated behaviorgetAuthConfigmock +resolveOrgProjectFromArgmock in log/list tests3. Dead code tests (PR #610)
PR #610 moved org resolution and DSN detection from
createSentryProjecttowizard-runner'sresolvePreSpinnerOptions. Tests that exercised those flows throughhandleLocalOphit the new!options.orgguard.Fix: Rewrote 15 tests to call
resolveOrgSluganddetectExistingProjectdirectly, plus 2 new tests forcreateSentryProject's existing-project check. No test coverage lost.4. Mtime test flakiness
Replaced
Bun.sleep(10)with explicitutimes()calls in dsn-cache and project-root-cache tests for deterministic mtime differences on Linux CI.Test plan
bun test— 630 pass, 4 fail (pre-existing: 3 resolve-target timeouts from unmocked fetch, 1 log/list failure from missing generated api-schema.json)bun run lint— clean (1 pre-existing warning)Made with Cursor