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

jetty 12.0.x no need test jar #12264

Closed
wants to merge 3 commits into from
Closed

Conversation

olamy
Copy link
Member

@olamy olamy commented Sep 12, 2024

  • simplify pom and avoid using test-jar for distribution test
  • spotless:apply

Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
Copy link
Contributor

@sbordet sbordet left a comment

Choose a reason for hiding this comment

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

I don't particularly like to put test classes in src/main, they belong to src/test.

If I need to reuse test classes, then it's natural to depend on a test-jar.

What is the problem this PR is trying to solve?

@olamy
Copy link
Member Author

olamy commented Sep 12, 2024

I don't particularly like to put test classes in src/main, they belong to src/test.

I do not see a problem with having this abstract class here (it's not a test class at all)
This module is intended to be used for testing (see the name ) only, and this class is abstract.
And we shared classes (all other tests) which are definitely not needed by other modules (we are polluting the classpath for no reason except one class)

If I need to reuse test classes, then it's natural to depend on a test-jar.

What is the problem this PR is trying to solve?

I want to avoid producing this test jar which is not needed and add more artifact (with not needed classes) for no reason

And you cannot use -Dmaven.test.skip=true

@olamy
Copy link
Member Author

olamy commented Sep 12, 2024

@sbordet :) #3080

@olamy
Copy link
Member Author

olamy commented Sep 13, 2024

more seriously I'm looking at improving build time and IO usage (disk) is a big problem.
So reducing the size of generated/copied file might help (here we can reduce the need of sharing a jar file with only 1 class)

By the way I wonder why do we have all of those test-ee9-distribution test-ee10-distribution with duplicated classes and test with only differences some module names such ee10-webapp vs ee9-webapp those could test parameter using ParameterizedTest
Reducing this will reduce the number of modules, and so IO/build time.

…jar and dependency not really needed

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
Copy link
Member Author

olamy commented Sep 14, 2024

finally moving AbstractJettyHomeTest.java is even better as it reduce inter dependencies (for only this single class), no more need of this test-jar as test classes were not re use but only this abstract class doing absolutely no test but just an helper as all other classes from jetty-tester module.

@olamy
Copy link
Member Author

olamy commented Sep 17, 2024

done via #12274

@olamy olamy closed this Sep 17, 2024
@olamy olamy deleted the jetty-12.0.x-no-need-test-jar branch September 17, 2024 10:54
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