-
Notifications
You must be signed in to change notification settings - Fork 542
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
[SUREFIRE-2152] Include name of JUnit5 templated-tests in test description #615
base: master
Are you sure you want to change the base?
Conversation
…displayName Ensures that all JUnit5 templated tests have a unique name that includes their invocation ID (index) This change also covers an issue of SUREFIRE-2087 where certain tests (namely, tests defined through a @testtemplate) still cause mixups of test invocations in relation with rerunFailingTestsCount, which can be mixed up by surefire if their name isn't unique for each distinct invocation. For an example description of this issue, see also: https://issues.apache.org/jira/browse/SUREFIRE-2087#comment-17690951
…iter It seems cleaner to strip the suffix of templated tests again here, since it is unrelated to the actual method signature, and might hence just cause confusion in context of a StackTraceWriter.
…hodDesc This includes the displayName (e.g. value of name="..." of a @ParameterizedTest) in the method description / test identifier. Provides more information than merely the invocation ID in case of failing tests, also for the console reporter. This can (especially if the actual list of arguments are quite large and/or dynamically determined at runtime) help a lot to identify which actual cases are the problematic ones.
… changes Since the name of templated tests now includes the displayName as suffix, the nameText is the same (and hence set to null) in some test-cases
Please check result of ITs - and fix ITs or code according |
I'd be happy to, but I'm unable to run the ITs locally to do this in the first place (as mentioned above in my initial description) Couple of things I did:
Not to speak of trying to get specific tests to run in IntelliJ to get them fixed (I have a different set of issues here) - but would be nice to at least be able to run things from commandline via mvn. So I'm kind of stuck on that end, tips / hints anyone? |
Can you show me more logs for your erro:
Which plugin was executed for it? |
Sure, here is some more log-output if that helps. Note that I once more ran this on a master / 208eae2 (git-cleaned, and with
Version / Java - info as well if relevant:
|
very strange ... when I build from master I have different plugin versions:
so I assume that you have not actual code in your workspace Please rebase and resolve conflicts. |
I'd rebase the branch of this PR once I'm able to run the ITs in some form (as said that's not working on master either for me at all, would need to sort this out first). And for running the ITs, I tried the same command again on latest master state (2709f76), same result / same failure. I also tried running it again with Can this be related to the maven version? |
OK I have the same error in Maven 3.8.2 - so please upgrade Maven first |
@s-rwe Do you want to pick this up again? |
Suggestion to support SUREFIRE-2152, helping developers identify problematic invocations of templated tests much quicker, especially also based on the console output.
The initial commit a87457b to handle JUnit5 templated tests quite separately in
RunListenerAdapter
also fixes another issue mentioned recently on SUREFIRE-2087, namely this one:I could assume that especially ff69448 might be controversial in this, since it mixes up the separation between the
methodDesc
andmethodDisp
(introduced in SUREFIRE-1546, AFAICS) to some extent. However, I would argue that it's quite important to be able to see the display-name of templated tests in general (i.e., in particular also in the default output of the console reporter), else it can be tedious to figure out more details.Still, it might be useful to make the changes of ff69448 conditional of a configuration flag - i.e. keeping it disabled by default and have a flag for opt-in (I don't really know how a config-flag would need to be done, help welcome if you'd consider that useful too)
Validation:
mvn clean install
is successfulmvn -Prun-its clean install