Address CodeRabbit test-quality findings#468
Conversation
All test-side hardening only — no production code changes. The bigger production findings (auto_queue/path_pairs/config/integrations error handling and ordering) are deferred for individual PRs with proper test coverage rather than bundled here. - option.component.spec.ts: add suite-level afterEach(useRealTimers) so a failing assertion inside vi.useFakeTimers() can't leak fake timers into the next test. Removed the per-test useRealTimers() calls that the new afterEach makes redundant. - path-pairs.service.spec.ts: rename "preserve existing list when refresh fails" to "clear pairs when refresh fails (catchError emits empty array)" — the test body always asserted [] so the old name was contradictory. - version-check.service.spec.ts: import packageJson and use 'v'+packageJson.version in the same-version test instead of hardcoding 'v0.17.0'. Future version bumps no longer break this test. - integrations.spec.ts: fail fast on API errors in beforeEach and afterEach (GET + each DELETE). Stops the test with a clear error instead of silently proceeding against unknown state. - settings.spec.ts: drop redundant field.clear() calls (3 sites). fill() already clears before typing; calling clear() separately races with Angular's signal-driven re-render. Pattern matches the existing fix in "text field change saves to backend". - test_context.py / test_web_app_job.py: stop attaching a StreamHandler in _make_context. assertLogs() in the tests installs its own capture handler, so the addHandler call only leaked across tests (loggers are singletons). Drops the now-unused import sys too. - test_active_scanner.py: register self.addCleanup(scanner.close) right after each ActiveScanner construction so multiprocessing resources always get closed even if assertions raise. Trailing scanner.close() calls left in place — Queue.close() is idempotent. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Minor bump for the develop → master sync. New user-facing functionality (LFTP hot-reload + differentiated restart notifications) and a non-trivial operational improvement (image size from 114 MB to 64 MB) warrant the minor over a patch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Test-only hardening from the CodeRabbit review on PR #467 (develop → master). Once this lands on develop, PR #467 picks up all of these automatically.
Acted on (8)
option.component.spec.tsafterEach(useRealTimers)so failing assertions can't leak fake timers across testspath-pairs.service.spec.ts[]version-check.service.spec.tspackageJsonand use'v'+packageJson.versioninstead of hardcoded'v0.17.0'integrations.spec.tsbeforeEach/afterEach(GET + each DELETE)settings.spec.tsfield.clear()calls —fill()already clears, separateclear()races with Angular signal re-rendertest_context.py_make_context(assertLogsinstalls its own capture handler)test_active_scanner.pyself.addCleanup(scanner.close)right after each construction so multiprocessing resources always close on assertion failuretest_web_app_job.pySkipped (with reasons)
option.component.ts(exportDEBOUNCE_TIME_MS)1000in the spec hasn't drifted in years.docker-image/Dockerfile(pinpython:3.13-alpine→python:3.13-alpine3.23)test-image/Dockerfile(pinuv:0.11)test_status.py(resp.jsoninstead ofjson.loads(str(resp.html)))auto_queue.py(try/except aroundto_file)config.py(copy-then-write atomicity)integrations.py(persistence ordering)path_pairs.py(try/except aroundto_file)package.jsonversion bump0.17.1(patch) vs0.18.0(minor — defensible given hot-reload and image-size work in this batch).Validation
py_compile; TS diagnostics show only pre-existing issues at unrelated lines)integrations.spec.tschange is the riskiest — fail-fast may surface latent issues, which is the point. If it does, that's a real bug we want to know about.🤖 Generated with Claude Code