[SUREFIRE-1266] Specifying groups shouldn't require junit on classpath if no test classes

[jira-importer]

Anders Hammar opened SUREFIRE-1266 and commented

Test framework: junit 4.8+
I have a scenario where we want to configure the groups parameter in a parent so that it doesn't have to be specified in all modules. However, not all modules have test classes and therefore no dependency on junit is specified in those projects. What then happens is that m-surefire-p fails with:

groups/excludedGroups require TestNG or JUnit48+ on project test classpath

This can be worked around by specifying skipTests on those projects, but it's a bit cumbersome.

It would be nice if surefire could detect that there are no test classes (src/test/main is empty) and therefore the project should just be ignored. However, I guess there could be test classes in a dependency so that might not be as easy.
Possibly the solution could be to not fail the surefire execution if groups/excludedGroups is set but TestNG or JUnit 4.8+ is not on the classpath, but issue a warning message and ignoring it?

---

Affects: 2.19.1

Remote Links:

• GitHub Pull Request #388
  

0 votes, 6 watchers

[jira-importer]

Tibor Digana commented

Anders Hammar
You can open PR in github, but the solution proposal should be
IF testClassesDirectory does not exist or is empty AND other-classpath is empty THEN skip plugin.
The other-classpath is integesting, see this http://maven.apache.org/surefire/maven-surefire-plugin/examples/configuring-classpath.html
See AbstractSurefireMojo.java but I am not sure if this is so easy to do.
In can of m-failsafe-p skipping the plugin should be preceded by cleanup of report; otherwise the verify phase would run using old reports.

[jira-importer]

Elliotte Rusty Harold commented

is it that hard to put junit on the classpath?

[jira-importer]

Ciprian Ciubotariu commented

I have hit this situation too, exactly as described by the author of this ticket. I have prepared a fix for it, together with a demo project. I think this is worth including in the plugin, for the following reasons:

1. It follows the principle of least surprise, since it matches the same expectation that surefire promotes when groups are disabled, i.e it skips projects that don't have any tests, thus they don't have a junit dependency.
2. It eliminates the imposition of an unneeded policy over our code, i.e adding a dependency that won't be used anyway if the project does not have any tests. Given the code, this is the result of an abstraction leak, due to the way surefire verifies parameters before deciding if the project does have any tests, or if those tests require junit/test-ng.

The existing code does check that the test directory does exist, and disabled the verification of the junit dependency if the folder is missing. Annoyingly, IDEs tend to create empty test directories that linger within the project, and getting teams of people to neatly remove them seems like a low value use of time.

I will continue on with the pull request

[jira-importer]

Ciprian Ciubotariu commented

I have created github PR #388 - please instruct me if you want me to open a new issue on this subject.

[jira-importer]

Slawomir Jaranowski commented

Why closed with won't fix ...

I will try resolve this issue in another way - maybe more generic.

We have method org.apache.maven.plugin.surefire.AbstractSurefireMojo#createProviders which detect providers (such test framework is available on project classpath),
I will run this method first and when have empty provider list don't do any other action such searching test classes and so on ...
 

[jira-importer]

Ciprian Ciubotariu commented

Slawomir Jaranowski can you please point to the new Jira issue, so I can monitor when the fix goes in?

[jira-importer]

Ciprian Ciubotariu commented

Closed the PR following the won't fix resolution here

[jira-importer]

Tibor Digana commented

Ciprian Ciubotariu
Why you closed it?
Now we are free because there is a pending Release Vote on the mailing list.
I have noticed your ticket in the email.

[jira-importer]

Ciprian Ciubotariu commented

tibordigana I noticed the resolution here got to won't fix so i assumed the maintainers rejected it. I reopened the PR