Skip to content
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

Improve testability #53

Merged
merged 1 commit into from
Dec 10, 2023
Merged

Improve testability #53

merged 1 commit into from
Dec 10, 2023

Conversation

Postremus
Copy link
Contributor

One of the main problems with the current tests is, they are not executable in idea.
This makes them pretty difficult to debug if anything fails.

Therefore, here is a rewrite of the tests.

  • Now https://github.com/khmarbaise/maven-it-extension is used for execution of the mojo. This allows to execute tests in ide after a first mvn verify.
  • Most tests are however now simple unit tests, which directly point at JasperReportCompiler, and configuration is done using a simple object. This again simplifies testing in ide.
  • The only mojo test left right now tests that the parameters set in pom are correctly mapped to a JasperMojoConfiguration object.
  • Also add a test that the thread naming is done correctly.
  • This also allows us to update to junit-jupiter, and drop some legacy dependencies
  • Update logging to use slf4j, instead of passing the logger around

@Postremus Postremus marked this pull request as ready for review December 10, 2023 20:05
now uses maven itf for integration tests, and makes most tests simple unit tests
@Postremus Postremus merged commit 9092d12 into main Dec 10, 2023
1 check passed
@Postremus Postremus deleted the better-testability branch December 10, 2023 20:11
@@ -61,7 +60,7 @@ public Void call() throws Exception {
InputStream in = new BufferedInputStream(new FileInputStream(source))) {
JasperCompileManager.compileReportToStream(in, out);
if (verbose) {
log.info("Compiling " + source.getName());
LOGGER.info("Compiling " + source.getName());
Copy link

Choose a reason for hiding this comment

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

Maybe use "Parameterized Logging" for all changed or new loggers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review. Yes I agree. Now that we are using slf4j, lets also switch to parameterized logging. Follow up pr coming soon.

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