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

Deferred properties in surefire argLine cause problems in 2.6.2 #1824

Closed
reckart opened this issue Sep 4, 2024 · 27 comments · Fixed by #1827
Closed

Deferred properties in surefire argLine cause problems in 2.6.2 #1824

reckart opened this issue Sep 4, 2024 · 27 comments · Fixed by #1827

Comments

@reckart
Copy link

reckart commented Sep 4, 2024

Looks like the new support for argLine causes trouble. If one uses a deferred property in the argline like, this ends up unresolved in the JUnit run configuration.

E.g.

<argLine>-Xmx512 @{argLineExtra}</argLine>

will create an unrunnable test run configuration that fails with

Error: could not open `{argLineExtra}`
@reckart reckart changed the title @-properties in surefire argLine cause problems Deferred properties in surefire argLine cause problems Sep 4, 2024
@reckart reckart changed the title Deferred properties in surefire argLine cause problems Deferred properties in surefire argLine cause problems in 2.6.2 Sep 4, 2024
@ahoehma
Copy link

ahoehma commented Sep 5, 2024

Same here :)

I have this settings in my surefire-plugin:

<plugin>
          <groupId>org.apache.maven.plugins</groupId>
          <artifactId>maven-surefire-plugin</artifactId>
          <version>3.5.0</version>
          <executions>
            <execution>
              <id>default-test</id>
              <phase>test</phase>
              <goals>
                <goal>test</goal>
              </goals>
              <configuration>
                <!-- @formatter:off Avoid newlines inside argLine! -->
                <!-- Sets the VM argument line used when unit tests are run. See jacoco plugin -->
                <argLine>-Xshare:off -Djdk.net.URLClassPath.disableClassPathURLCheck=true @{jacoco.surefireArgLine}</argLine>
                <!-- @formatter:on Avoid newlines inside argLine! -->
                <printSummary>true</printSummary>
                <useUnlimitedThreads>false</useUnlimitedThreads>
                <perCoreThreadCount>false</perCoreThreadCount>
                <parallelOptimized>false</parallelOptimized>
                <threadCount>${test-thread-count}</threadCount>
                <parallel>${test-parallel}</parallel>
                <forkCount>${test-fork-count}</forkCount>
                <reuseForks>${test-fork-reuse}</reuseForks>
                <rerunFailingTestsCount>${test-rerun-count}</rerunFailingTestsCount>
                <skipAfterFailureCount>${test-skip-after-failure-count}</skipAfterFailureCount>
                <parallelTestsTimeoutForcedInSeconds>400</parallelTestsTimeoutForcedInSeconds>
                <parallelTestsTimeoutInSeconds>400</parallelTestsTimeoutInSeconds>
                <properties>
                  <property>
                    <name>dataproviderthreadcount</name>
                    <value>${test-testng-dataproviderthreadcount}</value>
                  </property>
                </properties>
                <includes>
                  <include>${test-includes}</include>
                </includes>
                <reportFormat>plain</reportFormat>
                <consoleOutputReporter>
                  <disable>true</disable>
                </consoleOutputReporter>
                <statelessTestsetInfoReporter implementation="org.apache.maven.plugin.surefire.extensions.junit5.JUnit5StatelessTestsetInfoTreeReporter">
                  <theme>ASCII</theme>
                  <printStacktraceOnError>true</printStacktraceOnError>
                  <printStacktraceOnFailure>true</printStacktraceOnFailure>
                  <printStdoutOnError>true</printStdoutOnError>
                  <printStdoutOnFailure>true</printStdoutOnFailure>
                  <printStdoutOnSuccess>false</printStdoutOnSuccess>
                  <printStderrOnError>true</printStderrOnError>
                  <printStderrOnFailure>true</printStderrOnFailure>
                  <printStderrOnSuccess>false</printStderrOnSuccess>
                </statelessTestsetInfoReporter>
              </configuration>
            </execution>
          </executions>
          <dependencies>
            <dependency>
              <groupId>org.junit.jupiter</groupId>
              <artifactId>junit-jupiter-engine</artifactId>
              <version>5.11.0</version>
            </dependency>
            <dependency>
              <groupId>me.fabriciorby</groupId>
              <artifactId>maven-surefire-junit5-tree-reporter</artifactId>
              <version>1.3.0</version>
            </dependency>
          </dependencies>
        </plugin>

And this jacoco-maven-plugin:

<plugin>
          <groupId>org.jacoco</groupId>
          <artifactId>jacoco-maven-plugin</artifactId>
          <version>0.8.12</version>
          <executions>
            <execution>
              <id>coverage-prepare-agent</id>
              <goals>
                <goal>prepare-agent</goal>
              </goals>
              <configuration>
                <!-- Sets the name of the property containing the settings for JaCoCo runtime agent. -->
                <propertyName>jacoco.surefireArgLine</propertyName>
              </configuration>
            </execution>
            <!-- Ensures that the code coverage report for unit/it tests is created after unit tests have been run. -->
            <execution>
              <id>coverage-report</id>
              <phase>verify</phase>
              <goals>
                <goal>report</goal>
              </goals>
              <configuration>
                <!-- Sets the output directory for the code coverage report. -->
                <outputDirectory>${project.reporting.outputDirectory}/jacoco</outputDirectory>
              </configuration>
            </execution>
          </executions>
          <configuration>
            <skip>${test-coverage-skip}</skip>
          </configuration>
        </plugin>

This properties:

<properties>
    <resource.delimiter>@</resource.delimiter>
    <jacoco.surefireArgLine />
    <test-includes>**/*Test.java</test-includes>
    <test-parallel>classes</test-parallel>
    <test-thread-count>1</test-thread-count>
    <test-fork-count>2</test-fork-count>
    <test-fork-reuse>false</test-fork-reuse>
    <test-testng-dataproviderthreadcount>1</test-testng-dataproviderthreadcount>
    <test-rerun-count>1</test-rerun-count>
    <test-skip-after-failure-count>0</test-skip-after-failure-count><!-- set to 1 to fail fast -->
    <test-coverage-skip>false</test-coverage-skip>
</properties>

And now when I want to run a junit-test in eclipse (Alt+Shift+X, T) ... then the launcher for test have this vm arguments:

-ea
-Xshare:off -Djdk.net.URLClassPath.disableClassPathURLCheck=true @{jacoco.surefireArgLine}

And the run itself crash with

Error: could not open `{jacoco.surefireArgLine}'

Until yet I think that was working because the surefire argLine was not injected by m2e. Or?

But even if the new way is to inject this all the time, that would be fine for me, but then I need a way to override somehow the dynamic part of '@{jacoco.surefireArgLine}' which works only in a normal maven build.

@HannesWell
Copy link
Contributor

Until yet I think that was working because the surefire argLine was not injected by m2e. Or?

That's right. It looks like a regression from adding this feature in #1672.
Initially it was also intended to resolve these variables, but in order to make the review of that new feature manageable that part was left out. There was also not yet a solution that was fully satisfying.

As a workaround you can disable it in the meantime by launching Eclipse with the JVM system property
-Dm2e.process.test.configuration=false.

private static final boolean FEATURE_ENABLED = Boolean
.parseBoolean(System.getProperty("m2e.process.test.configuration", "true"));

@treilhes
Copy link
Contributor

treilhes commented Sep 8, 2024

Hello @reckart, @ahoehma, @HannesWell

@HannesWell : i don't know if you remember but my first version of #1672 was executing a list of plugins provided inside a custom property in order to generate variables created by those plugins.

This sub feature was removed and it was the right choice because m2e already have this feature available.

I won't be able to provide a sample conf for @reckart case as i don't know from which plugin argLineExtra comes from but
for @ahoehma case the property comes from the jacoco maven plugin (goal: prepare-agent).

So we need to flag this plugin as needed for the configuration phase by customizing the plugin execution with

<action>
       <execute>
              <runOnConfiguration>true</runOnConfiguration>
       </execute>
 </action>

@ahoehma can you try to add this lifecycle configuration into you pom.xml

<project>
    <build>
     .........
     <pluginManagement>
            <plugins>
                <plugin>
                    <groupId>org.eclipse.m2e</groupId>
                    <artifactId>lifecycle-mapping</artifactId>
                    <version>1.0.0</version>
                    <configuration>
                        <lifecycleMappingMetadata>
                            <pluginExecutions>
                                <pluginExecution>
                                    <pluginExecutionFilter>
                                        <groupId>org.jacoco</groupId>
                                        <artifactId>jacoco-maven-plugin</artifactId>
                                        <versionRange>[0.0.0,)</versionRange>
                                        <goals>
                                            <goal>prepare-agent</goal>
                                        </goals>
                                    </pluginExecutionFilter>
                                    <action>
                                        <execute>
                                            <runOnConfiguration>true</runOnConfiguration>
                                        </execute>
                                    </action>
                                </pluginExecution>
                            </pluginExecutions>
                        </lifecycleMappingMetadata>
                    </configuration>
                </plugin>
            </plugins>
        </pluginManagement>
     .....
    </build>
</project>

and replace @ by $ into argLine

replace
<argLine>-Xshare:off -Djdk.net.URLClassPath.disableClassPathURLCheck=true @{jacoco.surefireArgLine}</argLine>
by
<argLine>-Xshare:off -Djdk.net.URLClassPath.disableClassPathURLCheck=true ${jacoco.surefireArgLine}</argLine>

On my side it worked, what about you @ahoehma ?

@reckart
Copy link
Author

reckart commented Sep 8, 2024

@treilhes in my case, the value doesn't come from a plugin at all. It comes from a property manually set in the properties section of the POM.

@treilhes
Copy link
Contributor

treilhes commented Sep 8, 2024

@reckart So can you try replacing @{argLineExtra} by ${argLineExtra}, i think it will just work in this case

@reckart
Copy link
Author

reckart commented Sep 8, 2024

@treilhes maybe - but when I run this normally via the CLI, the deferred property is also properly resolved when a @{...} is used. Why should m2e behave differently and only resolve them for properties set by plugins?

@laeubi
Copy link
Member

laeubi commented Sep 8, 2024

Why should m2e behave differently and only resolve them for properties set by plugins?

Because m2e is not a CLI invocation ... a deferred property makes no sense given that this is evaluated at random times (e.g. when a project is updated) maybe when you create the launch config or ... So this looks like waiting for bad things to happen.

So basically in such case one needs to (re) evaluate it every time you run the JUnit, something that is not that trivial as it sounds.

@reckart
Copy link
Author

reckart commented Sep 8, 2024

I still don't get it. If you go through all the trouble of resolving deferred properties against dynamically set properties values for plugins like jacoco, why not resolve them also against regular properties?

@treilhes
Copy link
Contributor

treilhes commented Sep 8, 2024

@reckart variable with leading @ isn't a core maven feature but some kind of workaround used by failsafe/surefire plugins to load dynamicaly set variables during the build as maven replace variable leading with a $ during pom loading (so before any execution).

In your case, i think using @ is unneeded as your variable is a constant set in standard maven properties element

Your point is still valid for dynamic variable and i was thinking running @ahoehma case will successfully run in eclipse configuration but fail using cli but i did launch some test project and it runs successfully in eclipse and in cli with variable starting with $.

I'm a bit confused now, maybe some fix in failsafe/surefire was done removing the need for @

@laeubi
Copy link
Member

laeubi commented Sep 8, 2024

I still don't get it. If you go through all the trouble of resolving deferred properties against dynamically set properties values for plugins like jacoco, why not resolve them also against regular properties?

"deferred properties" are not supported at all.... see @treilhes comment(s).

@treilhes
Copy link
Contributor

treilhes commented Sep 8, 2024

@reckart : can you confirm your use case works (or not) in eclipse and in cli using $ ?

@reckart
Copy link
Author

reckart commented Sep 9, 2024

"deferred properties" are not supported at all.... see @treilhes comment(s).

Ok, got it.

I can probably replace the properties I have with the ${...} variant.

However, when using JaCoCo, this will not work. If I would replace the @{jacoco.argLine} with a ${jacoco.argLine} and tell m2e to run the prepare-agent, it might start working in m2e builds, but it won't work anymore in CLI builds, right?

@treilhes
Copy link
Contributor

treilhes commented Sep 9, 2024

That's what i was expecting also but contrary to my assumption it worked with ${jacoco.argLine}
If this use case is available on your side, can you use ${jacoco.argLine} instead of @{jacoco.argLine} and confirm if it works (or not) on your side using cli?

@reckart
Copy link
Author

reckart commented Sep 9, 2024

@treilhes For me, when I use ${jacoco.argLine} instead of @{jacoco.argLine} in a CLI build, I do not get any JaCoCo reports. I tried it with surefire 3.4.0 as well as with the latest 3.5.0 using Maven 3.9.7.

Just for good measure, I also tried with Maven 4.0.0-beta-4 - but does also not work.

Assuming ${jacoco.argLine} is properly resolved when the prepare-agent goal is executed in m2e (which I didn't test) - how about then to simply treat the @{...} properties like ${...} (in the surefire/failsafe support, not globally) in m2e so we can still stick with @{...} to keep the CLI builds working?

@treilhes
Copy link
Contributor

treilhes commented Sep 9, 2024

It's not as easy as you think, nothing was specificaly done to resolve variables, they just already were when accessing the argLine property.

Can you share a simplified version of your failing project as mine seems to work well ?

@gilyen
Copy link

gilyen commented Sep 9, 2024

As a workaround you can disable it in the meantime by launching Eclipse with the JVM system property -Dm2e.process.test.configuration=false.

Thank you, at least I can run again unit tests from eclipse with this ini setting - after I prematuraly updated my m2e plugin. I think this is the solution for most eclipse users until plugin is fixed in newer release.

@reckart
Copy link
Author

reckart commented Sep 9, 2024

Can you share a simplified version of your failing project as mine seems to work well ?

@treilhes sure - built you a new project from scratch to demonstrate.

demo.zip

With <argLine>${jacoco.argLine}</argLine> (pom.xml line 26), you get:

[INFO] --- jacoco:0.8.12:report (default-report) @ test ---
[INFO] Skipping JaCoCo execution due to missing execution data file.

With <argLine>@{jacoco.argLine}</argLine>, you get:

[INFO] --- jacoco:0.8.12:report (default-report) @ test ---
[INFO] Loading execution data file /../test/target/jacoco.exec
[INFO] Analyzed bundle 'test' with 1 classes

@treilhes
Copy link
Contributor

treilhes commented Sep 9, 2024

PR done #1827

@ahoehma
Copy link

ahoehma commented Sep 10, 2024

Hi guys, for me the whole argLine is not really relevant inside eclipse. More specific the "jacoco.argLine" I don't really need when I run tests inside eclipse ... because for that I'm using "EclEmma" :)

For me it would be fine if I can define "somehwere"

  • to ignore @{jacoco.argLine} or
  • define @{jacoco.argLine} with "" so that eclipse junit can run

@reckart
Copy link
Author

reckart commented Sep 10, 2024

I usually have JaCoCo configured in an optional profile that I activate using -Pjacoco on CLI builds.
However, because I have multiple properties that contribute the surefire argline, I have to set up default values for all properties, including jacoco.argLine because otherwise I would get an unresolved @{jacoco.argLine} when surefire is executed without the JaCoCo profile.

So IMHO the same should work in Eclipse. Assuming you do not configure theprepare-agent goal to be executed during m2e builds, it should then use the default value for jacoco.argLine defined in the POM's properties section - which I typically simply set to be empty.

I believe additional mechanism to ignore @{jacoco.argLine} should not be necessary.

That said, for people who always run JaCoCo during CLI builds and have not set up an empty jacoco.argLine in their properties section, it might be more convenient if m2e simply ignored these deferred properties if they are not set instead of keeping them unresolved.

@treilhes
Copy link
Contributor

Surefire/failsafe does not ignore unset property @{x} and fail so it seems logic for m2e to do the same in order to have the same behaviour in eclipse than in cli (which is by the way the initial requirement of this issue)

@G-Ork
Copy link
Contributor

G-Ork commented Sep 30, 2024

hi, for my eclipse Version: 2024-09 (4.33.0) - M2E 2.6.2.20240828-1954 none of the mentioned workarounds works. I've put -Dm2e.process.test.configuration=false

into my eclipse.ini. Also changed:
<argLine>@{surefireArgLine}</argLine>
to
<argLine>${surefireArgLine}</argLine>.

I've verified the system property on about dialog an is set to false.
But still got
Error: could not open {surefireArgLine}'`

@reckart
Copy link
Author

reckart commented Sep 30, 2024

Did you Maven -> Update project configuration...? If not, try - it should regenerate the run line.

@G-Ork
Copy link
Contributor

G-Ork commented Sep 30, 2024

Surefire/failsafe does not ignore unset property @{x} and fail so it seems logic for m2e to do the same in order to have the same behaviour in eclipse than in cli (which is by the way the initial requirement of this issue)

Realy a bad idea as long as you are not resolving the expressions as on cli.
Failing means we are left with an useless IDE.

@G-Ork
Copy link
Contributor

G-Ork commented Sep 30, 2024

Did you Maven -> Update project configuration...? If not, try - it should regenerate the run line.

My Problem was that the run-configurations where not cleaned up. I've updated the project config multiple times. But the old @{surefireArgLine} where still present in the run-configs of the unit-tests. Looks like the update feature is one-way.

@brychcy
Copy link
Contributor

brychcy commented Oct 2, 2024

I also stumbled over this.

Maybe the new feature could simply be automatically be disabled if the argLine contains "@{"?

@cvgaviao
Copy link

@reckart So can you try replacing @{argLineExtra} by ${argLineExtra}, i think it will just work in this case

https://maven.apache.org/surefire/maven-surefire-plugin/faq.html#late-property-evaluation

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 a pull request may close this issue.

9 participants