SONARJAVA-6208 S2699: Add approve to assertion method name pattern#5570
SONARJAVA-6208 S2699: Add approve to assertion method name pattern#5570NoemieBenard merged 7 commits intomasterfrom
Conversation
SummaryThis PR extends the S2699 rule ("Tests should include assertions") to recognize assertion calls from the ApproveJ library. ApproveJ uses an The change adds "approve" as an additional assertion method prefix to the existing pattern that already recognizes assert, verify, fail, should, check, expect, validate, andExpect, and others. A new test file with ApproveJ usage examples is included, along with documentation updates listing ApproveJ as a supported framework. What reviewers should knowWhere to start: Review the core change in Test coverage: The new Metrics update: The false negatives count in Why this matters: ApproveJ is a snapshot testing library that provides approval-style assertions. Without recognizing
|
Co-authored-by: sonar-review-alpha[bot] <266116024+sonar-review-alpha[bot]@users.noreply.github.com>
tomasz-tylenda-sonarsource
left a comment
There was a problem hiding this comment.
Overall this change looks good, but we should also update the documentation in RSpec. Let me know when you have time and I'll show you how to do this.
| <artifactId>core</artifactId> | ||
| <version>1.6.0</version> | ||
| <scope>test</scope> | ||
| </dependency> |
There was a problem hiding this comment.
Please fix formatting.
| "hasTruePositives": true, | ||
| "falseNegatives": 157, | ||
| "falseNegatives": 158, | ||
| "falsePositives": 1 |
There was a problem hiding this comment.
The false negatives count went up by 1 (157 → 158) after adding approve to the pattern. This suggests the autoscan corpus contains at least one test method that calls something like approveOrder() or approvePayment() — a business-logic domain method that the rule now silently treats as an assertion, causing it to miss a genuine missing-assertion issue.
The word approve is broad enough to appear in non-assertion domain methods inside test code. Is this increase expected/acceptable as a deliberate trade-off, or is it a signal that the pattern should be narrowed (e.g. via a type-based check anchored to org.approvej, similar to how other frameworks are handled)?
- Mark as noise
|





Add approve keyword to the assertion methods pattern to support ApproveJ library