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

Move jetty maven plugin integration tests out to tests module #11806

Open
janbartel opened this issue May 17, 2024 · 4 comments
Open

Move jetty maven plugin integration tests out to tests module #11806

janbartel opened this issue May 17, 2024 · 4 comments
Assignees
Labels
Bug For general bugs on Jetty side Build

Comments

@janbartel
Copy link
Contributor

jetty-12

We should move the integration tests for the jetty maven plugin out of each of the jetty-eeX-maven-plugin subprojects, and move it into submodules of tests instead. This will solve a nasty circular dependency, where the plugin integration tests depend on jetty-home (which in turn depends on each of the eeX subprojects).

@janbartel janbartel added Bug For general bugs on Jetty side Build labels May 17, 2024
@janbartel janbartel self-assigned this May 17, 2024
@janbartel
Copy link
Contributor Author

@olamy any comments?

@olamy
Copy link
Member

olamy commented Jun 2, 2024

My comment is a nasty circular dependency this is not nasty nor circular but a real dependency :)
To be honest, I think it's a lot of work for minor/none improvement. I would not even see this as a improvement as it will move some test files in another place of the source tree of the maven plugins code and by the way every maven plugins test will still have dependency on jetty-home.
And with the cache, the build is really fast now when testing this locally or via PR.
So, personally, I would not change that.

@janbartel
Copy link
Contributor Author

@olamy there is a circularity via the various eex-home modules: jetty-eex-maven-plugin -> jetty-home -> jetty-eex-home -> jetty-eex-maven-plugin.
My other thought is that we have the tests/test-integration module where we do all kinds of integration tests, so why not the plugin too?

@olamy
Copy link
Member

olamy commented Jun 3, 2024

I cannot see such dependency jetty-eex-home -> jetty-eex-maven-plugin. (from here https://github.com/jetty/jetty.project/blob/e1c7a7ca02a913b7def99af8223ad4e2855425a6/jetty-ee10/jetty-ee10-home/pom.xml)

If you want to do it for consistency, just do it. I just find it extra work for no gain but those new modules will still need dependency on jetty-home and so when we will want to test any change with maven plugins the build (up to jetty-home will be the same)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Build
Projects
None yet
Development

No branches or pull requests

2 participants