avoid deep pattern for maven-invoker-plugin generated junit files#307
avoid deep pattern for maven-invoker-plugin generated junit files#307olamy wants to merge 2 commits intojenkins-infra:masterfrom
Conversation
|
Hello 👋 Could you explain the why/what of this change? It's not clear what problem it is solving? |
| infra.runMaven(mavenOptions, jdk, null, null, addToolEnv) | ||
| } finally { | ||
| if (!skipTests) { | ||
| junit('**/target/surefire-reports/**/*.xml,**/target/failsafe-reports/**/*.xml,**/target/invoker-reports/**/*.xml') |
There was a problem hiding this comment.
If I understand correctly this change, the junit reports that would be in nested directories (for instance target/surefire-reports/foo/test-1.xml) would not be selected anymore.
Am I correct?
There was a problem hiding this comment.
I don't think they are actually nested
There was a problem hiding this comment.
surefire xml files are only generated at the top level directory.
|
do not forget to update the unit test ;) |
Context: jenkinsci/plugin-pom#508 |
jglick
left a comment
There was a problem hiding this comment.
While I think this change is correct, I doubt it will solve the problem in jenkinsci/plugin-pom#508 which IIUC was not about target/surefire-reports/some/deep/subdir/TEST-Stuff.xml but about some/deep/subdir/target/surefire-reports/TEST-Stuff.xml. And switching to for example target/surefire-reports/*.xml would cause us to lose legitimate test results from multimodule repos.
the new pattern |
|
Right, so (AFAICT) this change should not break anything, but neither is it likely to fix the problem in jenkinsci/plugin-pom#508. |
|
Even if those deep scanning could be avoided, the PR is not needed anymore. |
|
jenkinsci/plugin-pom#508 (comment): maybe would be appropriate to look only one level deep? |
No description provided.