Skip to content

CodeQL 15: chore(tests): narrow catch(Exception) to specific types in test helpers#201

Open
rlorenzo wants to merge 3 commits into
mainfrom
codeql/15-pdf-select-conversion
Open

CodeQL 15: chore(tests): narrow catch(Exception) to specific types in test helpers#201
rlorenzo wants to merge 3 commits into
mainfrom
codeql/15-pdf-select-conversion

Conversation

@rlorenzo
Copy link
Copy Markdown
Contributor

@rlorenzo rlorenzo commented May 13, 2026

Summary

Closes 5 of the remaining cs/catch-of-all-exceptions alerts in test code by narrowing the catch to the specific types each test helper is actually expecting:

  • test/CTS/AssessmentControllerTest.cs:266,282 - IsForbidResult / IsNotFoundResult helpers convert xUnit assertion failures to false. Narrowed from catch (Exception) to catch (Xunit.Sdk.XunitException), which is exactly what Assert.X(...) throws.
  • test/ClinicalScheduler/EmailNotificationTest.cs:284,466 - diagnostic try/catch around RemoveInstructorScheduleAsync. Narrowed to when (ex is InvalidOperationException or DbUpdateException or SqlException or OperationCanceledException) - the actual exception families that production code throws (matches the service's when filter from CodeQL 5: refactor(ClinicalScheduler): catch specific exceptions in services #193).
  • test/ClinicalScheduler/RotationServiceTest.cs:98 - test-data-seeding try/catch around SaveChanges. Narrowed to when (ex is DbUpdateException or InvalidOperationException).

Context

Fifteenth in the CodeQL N: cleanup series. (The PDF/Excel cs/linq/missed-select Select-conversion attempted on this branch was reverted - the script over-matched loops where the iter var is also used in headers/totals/spacers, breaking compilation. Those remaining 19 alerts genuinely require the iter var and can't be cleanly converted.)

Test plan

  • npm run test:backend - 1946 tests passing
  • Pre-commit lint+test+verify all passed

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: db069067-4287-4b71-9891-2cff48f30106

📥 Commits

Reviewing files that changed from the base of the PR and between 38de1ad and 33c47fb.

📒 Files selected for processing (3)
  • test/CTS/AssessmentControllerTest.cs
  • test/ClinicalScheduler/EmailNotificationTest.cs
  • test/ClinicalScheduler/RotationServiceTest.cs

📝 Walkthrough

Walkthrough

Exception handling across three test files is narrowed from catching generic Exception to catching only specific, expected exception types. Test helpers now catch assertion failures only; test methods filter to database and operation exceptions; test data seeding filters to database exceptions.

Changes

Test exception handling narrowing

Layer / File(s) Summary
Test helper assertion narrowing
test/CTS/AssessmentControllerTest.cs
IsForbidResult and IsNotFoundResult helpers narrow their catch clause to Xunit.Sdk.XunitException, allowing non-assertion exceptions to propagate.
Email notification test exception filtering
test/ClinicalScheduler/EmailNotificationTest.cs
New Microsoft.Data.SqlClient import added; two test methods now catch only InvalidOperationException, DbUpdateException, SqlException, or OperationCanceledException instead of all exceptions.
Rotation service test data seeding exception filtering
test/ClinicalScheduler/RotationServiceTest.cs
SeedTestData exception handler narrowed to catch only DbUpdateException and InvalidOperationException, preserving logging and rethrow behavior while allowing other exceptions to propagate.

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: narrowing catch(Exception) to specific exception types in test helpers across three test files.
Description check ✅ Passed The description is directly related to the changeset, providing specific details about which test files were modified and how the exception handling was narrowed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

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

✨ 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 codeql/15-pdf-select-conversion

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.

@codecov-commenter
Copy link
Copy Markdown

Bundle Report

Bundle size has no change ✅

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.96%. Comparing base (5949e52) to head (c72b48f).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #201   +/-   ##
=======================================
  Coverage   42.96%   42.96%           
=======================================
  Files         877      877           
  Lines       51468    51468           
  Branches     4802     4802           
=======================================
  Hits        22113    22113           
  Misses      28831    28831           
  Partials      524      524           
Flag Coverage Δ
backend 43.04% <ø> (ø)
frontend 41.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…ilters

Removed 'Microsoft.EntityFrameworkCore.' and 'Microsoft.Data.SqlClient.'
prefixes from the catch when-filters added in codeql/15. Added
'using Microsoft.Data.SqlClient;' to EmailNotificationTest.
@rlorenzo
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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