-
Notifications
You must be signed in to change notification settings - Fork 0
test: avoid polluting test classpaths with sample dependencies to be scanned #4
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
Conversation
656b562 to
f673a4d
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
This PR restructures how sample artifacts used by tests are provided so they no longer pollute the test classpath, while updating tests and test infrastructure accordingly. It also tightens resource handling in the shared test base and refines the integration test around engine analysis errors.
Changes:
- Replace many hardcoded resource lookups with paths under
maven-lib/and configure the core module’smaven-dependency-pluginto copy specific sample artifacts intotarget/test-classes/maven-libinstead of declaring them as test-scoped dependencies. - Simplify and harden test infrastructure utilities (
BaseTest,EngineIT) by removing legacy logging setup, usingObjects.requireNonNullfor resource access, and asserting more precisely on expected analysis exceptions. - Clean up poms (
core,utils,maven) by removing now-unneeded test dependencies and adjustingdependency:analyzeconfiguration for test-only libraries.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/pom.xml | Adjusts maven-dependency-plugin usedDependencies configuration for test logging dependencies used by dependency:analyze-report. |
| maven/pom.xml | Removes explicit plexus utility dependencies that were overriding transitive versions, simplifying the Maven module’s dependency graph. |
| core/pom.xml | Removes legacy test-scoped sample dependencies and introduces a test-dependencies profile that copies specific sample artifacts into target/test-classes/maven-lib via maven-dependency-plugin. |
| core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionRuleTest.java | Updates suppression tests to load Spring/Struts sample JARs from the new maven-lib/ location. |
| core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionParserTest.java | Cleans up obsolete commented-out resource-loading code in suppression parser tests. |
| core/src/test/java/org/owasp/dependencycheck/reporting/ReportGeneratorIT.java | Switches report-generation IT to load Struts, WAR, commons-fileupload, and Axis2 test artifacts from maven-lib/ and removes old classloader-based lookups. |
| core/src/test/java/org/owasp/dependencycheck/dependency/DependencyTest.java | Points dependency checksum tests at the Struts sample JAR under maven-lib/. |
| core/src/test/java/org/owasp/dependencycheck/data/update/NvdApiDataSourceTest.java | Drops a jspecify @NonNull annotation and import, loosening a test helper signature. |
| core/src/test/java/org/owasp/dependencycheck/data/nvdcve/DriverLoaderTest.java | Cleans out unused commented resource-path code, relying solely on BaseTest.getResourceAsFile. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/VulnerabilitySuppressionAnalyzerIT.java | Updates vulnerability suppression IT to load commons-fileupload from maven-lib/. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/JarAnalyzerTest.java | Points JAR analyzer tests at Struts and Xalan JARs in maven-lib/ instead of the raw test-classes directory. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/HintAnalyzerTest.java | Loads the Guice sample JAR from maven-lib/ while leaving other resources unchanged. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/FileNameAnalyzerTest.java | Adjusts file-name analyzer tests to use Struts and Axis2 JARs from maven-lib/. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/DependencyMergingAnalyzerTest.java | Uses the aar-1.0.0.aar test artifact from maven-lib/ for Android dependency merging tests. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/CpeSuppressionAnalyzerIT.java | Updates CPE suppression IT to use the commons-fileupload JAR from maven-lib/. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/CPEAnalyzerIT.java | Switches several CPE analyzer tests to use maven-lib/ JARs for Spring context-support, ehcache, xstream, Struts, and Spring core. |
| core/src/test/java/org/owasp/dependencycheck/analyzer/ArchiveAnalyzerIT.java | Updates EAR-archive tests to load the Daytrader EAR from maven-lib/ and removes old commented resource path variants. |
| core/src/test/java/org/owasp/dependencycheck/EngineIT.java | Refactors the engine integration test to assert on unexpected analysis exceptions via Hamcrest, and adjusts imports and throws signatures. |
| core/src/test/java/org/owasp/dependencycheck/BaseTest.java | Simplifies test setup, removes legacy logging muting and Assume-based resource checks, and uses Objects.requireNonNull for resource lookup helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f673a4d to
6c27492
Compare
…scanned Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com>
6c27492 to
08199d0
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
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description of Change
Related issues
Have test cases been added to cover the new functionality?
yes/no