Skip to content

Added more unit tests for japicmp-maven-plugin#493

Merged
siom79 merged 7 commits intosiom79:masterfrom
bradleylarrick:plugin-unit-tests
Feb 9, 2026
Merged

Added more unit tests for japicmp-maven-plugin#493
siom79 merged 7 commits intosiom79:masterfrom
bradleylarrick:plugin-unit-tests

Conversation

@bradleylarrick
Copy link
Copy Markdown
Contributor

This is almost entirely new unit tests for the plugin - up to almost 85% coverage of instructions. Two deliverable classes were changed:

  • ConfigParameters: I added setters for a couple of properties in OverrideCompatibilityChangeParameter.
  • JApiCmpProcessor: I commented out some dead code related to semantic version levels (please confirm my logic). I also refactored resolveDependencyToFile to properly substitute property values in the systemPath defined in the Dependency.

I'm still working on a unit test that would have caught the problem found by the Apache httpcomponents team.

@bradleylarrick
Copy link
Copy Markdown
Contributor Author

I added a unit test that goes against the Apache httpcomponents project to test when an annotation is excluded.

Copy link
Copy Markdown
Owner

@siom79 siom79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work. I have two minor comments.

+ Joiner.on(',').join(
JApiSemanticVersionLevel.values()));
}
// This is dead code. foundSemanticVersionLevel is initialized above, and the for-loop
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this seems to be dead code. Can we remove it completely instead of commenting it out?

} else {
throw new MojoFailureException("Could not resolve property '" + property + "'.");
}
// Substitute any properties in the path with their values
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to extract this into a method with the name substituteProperties() or similiar?

@bradleylarrick
Copy link
Copy Markdown
Contributor Author

Done and Done.

@siom79 siom79 merged commit 8d8b23b into siom79:master Feb 9, 2026
6 checks passed
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