Skip to content

test: restore branch coverage threshold to 85%#118

Merged
BillChirico merged 2 commits intomainfrom
fix/coverage-85
Feb 28, 2026
Merged

test: restore branch coverage threshold to 85%#118
BillChirico merged 2 commits intomainfrom
fix/coverage-85

Conversation

@BillChirico
Copy link
Collaborator

Summary

Restores branch coverage threshold from 83% (temporary) back to 85% by adding targeted tests for low-branch modules.

Changes

  • vitest.config.js — branch threshold back to 85%
  • tests/modules/events.coverage.test.js — new coverage tests for events module branches
  • tests/sentry.init.test.js — new coverage tests for Sentry init branches

Coverage

Metric Before After
Branches 83.54% 85.07%
Statements 91.93%
Lines 92.75%

Closes #117

Copilot AI review requested due to automatic review settings February 28, 2026 01:11
@claude
Copy link

claude bot commented Feb 28, 2026

Claude finished @BillChirico's task in 1m 58s —— View job


Review: PR #118 — Restore branch coverage threshold to 85%

  • Read AGENTS.md for project conventions
  • Review changed files (vitest.config.js, events.coverage.test.js, sentry.init.test.js, use-moderation-cases.ts)
  • Check for code quality, architecture, testing, and security issues
  • Post inline comments for specific issues
  • Submit review verdict

Found 2 issues (1 warning, 1 nitpick):

# Severity File Issue
1 🟡 Warning tests/modules/events.coverage.test.js:82 Missing pollHandler.js mock — real module pulled in unlike sibling test file
2 🔵 Nitpick web/src/hooks/use-moderation-cases.ts:96-101 Dead useEffect after dep change to [] — should be removed entirely
AI fix prompt (copy-paste into Claude Code)
Fix these 2 issues in PR branch fix/coverage-85:

1. In tests/modules/events.coverage.test.js, add a missing vi.mock for pollHandler.js 
   before line 82 (before the imports). The existing tests/modules/events.test.js has 
   this mock at lines 36-39 — add it here too:
   
   vi.mock('../../src/modules/pollHandler.js', () => ({
     handlePollVote: vi.fn().mockResolvedValue(undefined),
   }));

2. In web/src/hooks/use-moderation-cases.ts, remove the dead useEffect at lines 96-101 
   entirely (the "Client-side sort toggle" comment and the useEffect block). After 
   changing its dependency from [sortDesc] to [], it runs once on mount when casesData 
   is null, making it a no-op. The sort direction is now handled by the fetch effect 
   which has sortDesc in its dependency array.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 2026

Warning

Rate limit exceeded

@BillChirico has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 22 minutes and 20 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between e049897 and b80a017.

📒 Files selected for processing (2)
  • tests/modules/events.coverage.test.js
  • web/src/hooks/use-moderation-cases.ts
📝 Walkthrough

Walkthrough

This PR introduces two new test suites to increase branch coverage from 83% to 85%, and updates the coverage threshold accordingly. Tests cover event handlers (message creation, reactions, reviews, challenges), reply detection, error handling, and Sentry initialization behavior across various environment configurations.

Changes

Cohort / File(s) Summary
Test Suites
tests/modules/events.coverage.test.js, tests/sentry.init.test.js
Adds comprehensive unit tests covering event subsystem handlers (ready, messageCreate, reactions, reviews, showcases, challenges), error paths, and Sentry initialization behavior under different environment variables and DSN configurations.
Coverage Configuration
vitest.config.js
Increases branch coverage threshold from 83% to 85%.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main objective: restoring the branch coverage threshold to 85% by adding tests.
Description check ✅ Passed The description clearly explains the purpose, lists all changes made, and provides concrete coverage metrics showing the improvement achieved.
Linked Issues check ✅ Passed The PR fully addresses issue #117 by adding comprehensive tests and restoring the branch coverage threshold from 83% to 85%.
Out of Scope Changes check ✅ Passed All changes are directly related to the coverage objective: test files for low-branch modules and the vitest config threshold adjustment.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/coverage-85

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 and usage tips.

@greptile-apps
Copy link

greptile-apps bot commented Feb 28, 2026

Greptile Summary

Restores branch coverage threshold from 83% to 85% by adding targeted coverage tests for the events module and Sentry initialization. Also fixes Biome exhaustive-deps lint warnings in use-moderation-cases.ts.

Key changes:

  • ✅ Branch coverage threshold increased to 85% in vitest.config.js
  • ✅ New comprehensive coverage tests for events module (511 lines) covering message handlers, reaction handlers, and error branches
  • ✅ New coverage tests for Sentry initialization covering DSN validation, environment fallbacks, and beforeSend filtering
  • ⚠️ Fixed exhaustive-deps warnings in use-moderation-cases.ts by adding sortDesc to dependencies, but introduced dead code in the client-side sort toggle effect (should be removed entirely)

Confidence Score: 3/5

  • Safe to merge with one logical issue that should be addressed
  • The PR achieves its stated goal of increasing branch coverage to 85% through well-structured tests. However, the change to use-moderation-cases.ts introduces dead code - a useEffect that only runs on mount when data is null, serving no purpose. This should be removed rather than just changing its dependency array to silence the linter.
  • web/src/hooks/use-moderation-cases.ts - contains dead code that should be removed

Important Files Changed

Filename Overview
vitest.config.js Restored branch coverage threshold from 83% to 85% as intended
tests/modules/events.coverage.test.js New coverage tests for events module - comprehensive branch coverage with minor formatting cleanup
tests/sentry.init.test.js New coverage tests for Sentry initialization - covers all configuration branches
web/src/hooks/use-moderation-cases.ts Fixed exhaustive-deps warnings but introduced dead code in client-side sort toggle

Last reviewed commit: b80a017

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 28, 2026
claude[bot]
claude bot previously approved these changes Feb 28, 2026
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean PR — zero issues found. Tests are well-structured, properly mock dependencies, and cover meaningful branches (events module handlers, Sentry init/beforeSend logic). Coverage threshold correctly restored to 85%.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores the project’s Vitest branch coverage gate back to 85% by adding targeted tests that exercise previously uncovered branches in low-coverage modules.

Changes:

  • Raised Vitest branch coverage threshold from 83% to 85%.
  • Added Sentry init tests to cover DSN/env/trace-rate and beforeSend filtering/scrubbing branches.
  • Added events-module coverage tests to hit a variety of early-return and error-handling branches.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

File Description
vitest.config.js Restores branch coverage threshold to 85%.
tests/sentry.init.test.js Adds targeted tests for src/sentry.js initialization and beforeSend branch behavior.
tests/modules/events.coverage.test.js Adds a coverage-focused suite to exercise multiple branches in src/modules/events.js.
package-lock.json Updates lockfile contents (deps/engine metadata) alongside the test/coverage changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 2026

Caution

Docstrings generation - FAILED

No docstrings were generated.

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues found:

  1. 🟡 Missing pollHandler.js mock (tests/modules/events.coverage.test.js:82) — The existing events.test.js mocks pollHandler.js, but this new test file doesn't. Since events.js statically imports it, the real module (and its dependencies) get pulled in, coupling the test to unrelated production code.

  2. 🔵 Dead useEffect (web/src/hooks/use-moderation-cases.ts:96-101) — After changing the dep array from [sortDesc] to [], this effect is a no-op (runs once on mount when casesData is null). Should be removed entirely since the fetch effect now handles sort direction.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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.

Restore branch coverage threshold to 85%

2 participants