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

Quarkus Platform should not ban test scope in endorsed third party test projects #3136

Closed
ppalaga opened this issue Jul 8, 2019 · 11 comments
Assignees
Labels
area/platform Issues related to definition and interaction with Quarkus Platform kind/bug Something isn't working
Milestone

Comments

@ppalaga
Copy link
Contributor

ppalaga commented Jul 8, 2019

This is a followup of apache/camel-quarkus#35 cc @lburgazzoli

Background: @aloubyansky 's proposal https://github.com/aloubyansky/moving-camel-out-of-quarkus/blob/master/it/camel-servlet/pom.xml#L26 how endorsed third party extension tests should look like bans <scope>test</scope> in integration test projects. This is motivated by the fact that @aloubyansky wants to be able to run these tests inside the planned Quarkus Platform build. Dependencies with <scope>test</scope> would not be resolved properly there.

We (camel-quarkus team) accepted the ban of test scope for now. However, we would not like this to be a definitive solution, because this makes our integration tests non-idiomatic and thus non-usable as quickstarts. It would be nice if Quarkus team could come with a solution that allows us keeping to use <scope>test</scope> in camel-quarkus.

@lburgazzoli
Copy link
Contributor

@aloubyansky does the removal of the test scope eventually impact the build time as test dependencies leak into the compile classpath ?

@aloubyansky
Copy link
Member

Right, they will end up in the lib dir.

@aloubyansky
Copy link
Member

aloubyansky commented Aug 7, 2019

That's a tricky one. The issue is that the test deps should be loadable by the surefire plugin. For that to work there are a few options:

  1. the way it is now, i.e. test scope should be banned for test artifacts;
  2. the test scope deps should be duplicated in the platform testsuite pom;
  3. some kind of classloading hack.

Personally, I find 2 is worse than 1. Test artifacts are supposed to be used in the testsuite anyway. So, using anything but test scope shouldn't be a big deal. This is how maven dependency resolution works. The issue is not specific to Quarkus, it's how external tests are supposed to be integrated in general, afaiu.

3 is not nice by definition, although that depends on the impl, I guess. Here is a hack that appears to work:

  • add a tmp dir to the surefire plugin as an additional classpath element;
  • add another bootstrap maven plugin (executed before surefire) which resolves the missing test deps and unpacks them into that tmp dir.

I also tried another hack - modifying the surefire configuration using Maven API during the build which didn't work (I guess for good).

@aloubyansky
Copy link
Member

The way I see it, the Maven way to import test JARs applied to Quarkus suggests separation of the test application (the app used in the tests) and the tests themselves by moving them into their own modules. So the test app would be developed in one module and the tests - in a different one which would depend on the test app artifact.
When Maven resolves dependencies of a project it does not include test deps of its direct deps, even if the direct dependency is a test jar. Which means if a test jar is to be imported by other projects, its deps that are required during testing shouldn't be test scoped.
It doesn't seem great, I'm just trying to figure out the pure Maven model to do this.

@ppalaga
Copy link
Contributor Author

ppalaga commented Aug 8, 2019

Thanks for the analysis, @aloubyansky . Let me check if we can hack our build process to modify the test jars as you need. Because I do not like the idea to have separate test modules on our side, I'd write a piece of groovy that generates a pom.xml for the tests jar as-if in a separate module and installs/deploys it together with the test jar under the coordinates of the synthetic module.

@lburgazzoli
Copy link
Contributor

I'm a little worried to have a script to workaround this issue on an external extension side for a number of reasons:

  1. each extension need to implement the very same workaround
  2. each extension need to maintain the script and be sure it is compatible with quarkus
  3. is is fragile

A better solution would be to have support from the quarkus tooling side so each extension can just use it and what happens it controlled by quarkus so you can roll-out fixes or implement additional feature easily

@ppalaga
Copy link
Contributor Author

ppalaga commented Aug 19, 2019

Thinking of it again, the generation of the POM for the synthetic tests module and installing it to the local maven repo [1] could be also run on the Quarkus side. That would indeed solve the problems named by @lburgazzoli above. WDYT, @aloubyansky ?

[1] apache/camel-quarkus#140

@maxandersen maxandersen added the area/platform Issues related to definition and interaction with Quarkus Platform label Sep 30, 2019
@paulrobinson paulrobinson added this to To do in Deprecated - Quarkus 1.0 via automation Oct 10, 2019
@maxandersen
Copy link
Contributor

@ppalaga
Copy link
Contributor Author

ppalaga commented Oct 11, 2019

Yes, quarkusio/quarkus-platform#7 is our proposal, how the current issue should be solved so closing this one.

@ppalaga ppalaga closed this as completed Oct 11, 2019
Quarkus Platform and Tooling automation moved this from To do to Done Oct 11, 2019
Deprecated - Quarkus 1.0 automation moved this from To do to Done Oct 11, 2019
@paulrobinson paulrobinson mentioned this issue Oct 15, 2019
18 tasks
@cescoffier cescoffier added the triage/wontfix This will not be worked on label Oct 15, 2019
@maxandersen
Copy link
Contributor

reopening as by accident issues were enabled on platform - no need to spread that thin across.

here is comment from @ppalaga on latest progress with title "Run third party integration tests ":

@aloubyansky agreed to try this proposal. cc @maxandersen

Third party side:

  • The third party projects are required to deploy standard test-jars for their integration tests to a public Maven repo, e.g. Central.

Quarkus Platform side:

@maxandersen maxandersen reopened this Oct 15, 2019
Quarkus Platform and Tooling automation moved this from Done to In progress Oct 15, 2019
Deprecated - Quarkus 1.0 automation moved this from Done to In progress Oct 15, 2019
@maxandersen maxandersen removed the triage/wontfix This will not be worked on label Oct 15, 2019
@aloubyansky aloubyansky added this to the 0.25.0 milestone Oct 22, 2019
@aloubyansky
Copy link
Member

There was an adjustment in Quarkus 0.25.0 to allow the integrating the tests described above.

Deprecated - Quarkus 1.0 automation moved this from In progress to Done Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform Issues related to definition and interaction with Quarkus Platform kind/bug Something isn't working
Projects
No open projects
Development

No branches or pull requests

5 participants