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

QuarkusIntegrationTest: argLine is also given to docker run, which doesn't work because jacoco sets -javaagent #19017

Closed
bdevreugd-vialis opened this issue Jul 27, 2021 · 6 comments · Fixed by #19029

Comments

@bdevreugd-vialis
Copy link
Contributor

In another issue the ability to set quarkus.test.arg-line was added, in order to make Jacoco work when running integration test using java executable.

However the argLine is also used for docker runs, which fails because '-javaagent' is not a valid docker argument.

Suggestion:
Instead of specifying one arg-line, add the ability to specifiy an arg-line per artifact type, so:

  • quarkus.test.jar.arg-line
  • quarkus.test.native.arg-line
  • quarkus.test.jar-container.arg-line
  • quarkus.test.native-container.arg-line

List<String> strings = config.getOptionalValues("quarkus.test.arg-line", String.class)

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 27, 2021

/cc @geoand

@geoand
Copy link
Contributor

geoand commented Jul 27, 2021

I don't want to revert back to that TBH.

What does your test scenario look like that involves running both the jar artifact and the docker artifact?

@bdevreugd-vialis
Copy link
Contributor Author

The POM is setup like: https://quarkus.io/guides/tests-with-coverage#coverage-for-integration-tests
Our CI runs two jobs:

  • one creates a jar artifact and runs integration test + sonar. Because it is an jar artifact, we also get code coverage.
  • the other job creates a docker image (either native or jvm) and also runs integration test.

This works fine on Quarkus 2.0, but fails on 2.1 since the argLine is being passed as docker argument. Since arguments being passed to java/native or docker are nothing alike, it doesn't make much sense to me to share them in one config property.

@geoand
Copy link
Contributor

geoand commented Jul 27, 2021

But you can fairly easily configure some Maven profiles so set the appropriate value for each environment you are building in.

I really don't plan on moving back to the previous state unless there is a very good reason to

@bdevreugd-vialis
Copy link
Contributor Author

Sure I can, it just doesn't feel right that a property has 3 different meanings (jvm argument, docker argument or quarkus native executable argument).

Ah well, at least change the documentation:

<configuration>
  <systemPropertyVariables>
    <java.util.logging.manager>org.jboss.logmanager.LogManager</java.util.logging.manager>
    <maven.home>${maven.home}</maven.home>
    <quarkus.test.argLine>${argLine}</quarkus.test.argLine>
    <!-- If your integration tests require a different profile, you can set that here as well.
      <quarkus.test.native-image-profile>it</quarkus.test.native-image-profile>
    -->
  </systemPropertyVariables>
</configuration>

https://github.com/quarkusio/quarkus/blame/main/docs/src/main/asciidoc/tests-with-coverage.adoc#L304-L307

Because following this, the native executable will be run with the jacoco javaagent.

@geoand
Copy link
Contributor

geoand commented Jul 27, 2021

Sure that sounds reasonable. Would you like to contribute that documentation fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants