-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Java: port quality query java/mocking-all-non-private-methods-means-unit-test-is-too-big
#20205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java: port quality query java/mocking-all-non-private-methods-means-unit-test-is-too-big
#20205
Conversation
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.ql
Fixed
Show fixed
Hide fixed
f4a5e1e
to
cdb0d25
Compare
…ansUnitTestIsTooBig.qlref`
Classes with only one public method should be compliant when mocked.
…s-too-big` to quality status
cdb0d25
to
ff648fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.ql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good! Only a few questions / comments.
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.md
Outdated
Show resolved
Hide resolved
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.ql
Outdated
Show resolved
Hide resolved
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.ql
Outdated
Show resolved
Hide resolved
java/ql/src/Likely Bugs/Frameworks/JUnit/MockingAllNonPrivateMethodsMeansUnitTestIsTooBig.ql
Outdated
Show resolved
Hide resolved
…test-is-too-big` to `java/excessive-public-method-mocking` and changed wording from non-private to public
467df42
to
b271f1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
java/ql/src/Likely Bugs/Frameworks/JUnit/ExcessivePublicMethodMocking.ql
Outdated
Show resolved
Hide resolved
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 <[email protected]>
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-big
query. Key changes: