Java: port quality query java/mocking-all-non-private-methods-means-unit-test-is-too-big#20205
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Napalys
force-pushed
the
java/mocking-all-non-private-methods-means-unit-test-is-too-big
branch
from
f4a5e1e to
cdb0d25
Compare
…ansUnitTestIsTooBig.qlref`
Classes with only one public method should be compliant when mocked.
…s-too-big` to quality status
Napalys
force-pushed
the
java/mocking-all-non-private-methods-means-unit-test-is-too-big
branch
from
cdb0d25 to
ff648fc
Compare
There was a problem hiding this comment.
Pull Request Overview
Ports the java/mocking-all-non-private-methods-means-unit-test-is-too-big query to detect unit tests that mock all public methods of a class, which may indicate the test is testing too much functionality.
Key changes:
- Added logic to exclude classes with only 1 public method from being flagged as violations
- Reduced false positive results from 1,332 to 250 on MRVA 1000
- Added comprehensive test stubs for Mockito 5.14 and JUnit 4.13 to support the query functionality
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.ql |
Main query implementation that detects excessive mocking patterns |
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.md |
Documentation explaining the rule and providing usage examples |
java/ql/test/query-tests/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig/ |
Test cases with expected results demonstrating compliant and non-compliant patterns |
java/ql/test/stubs/mockito-5.14/ |
Mockito framework stubs for testing support |
java/ql/test/stubs/junit-4.13/ |
JUnit 4.13 framework stubs for testing support |
java/ql/integration-tests/java/query-suite/*.qls.expected |
Updated query suite expectations to include the new rule |
michaelnebel
left a comment
michaelnebel
left a comment
There was a problem hiding this comment.
Overall this looks good! Only a few questions / comments.
…test-is-too-big` to `java/excessive-public-method-mocking` and changed wording from non-private to public
Napalys
force-pushed
the
java/mocking-all-non-private-methods-means-unit-test-is-too-big
branch
from
467df42 to
b271f1f
Compare
Co-authored-by: michaelnebel <michaelnebel@github.com>
owen-mc
left a comment
owen-mc
left a comment
There was a problem hiding this comment.
I don't think "Likely Bugs" is the best folder for this query.
My only other concern is with the autofixes. Do you think they are good enough to add this to show the autofixes to users? Can you think of any way of improving them, e.g. with more complete examples in the qhelp?
Fair point, I agree. The main reason I moved it there was because of the
I will try to think of something. 🤔 |
…Mocking.ql Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
|
I think `Violations of Best Practice" is the best high-level folder. None of the existing subfolders really match, so maybe create a new one called "Testing". It is an option to accept that autofixes aren't good for this query at the moment, and not add it to autofix. |
…too-big` to a more appropriate location, namely `Violation of Best Practice/Testing`
…nit-test-is-too-big
Ported the
java/mocking-all-non-private-methods-means-unit-test-is-too-bigquery. Key changes: