Maven Surefire

Make ignoring of <includes> and <excludes> on -Dtest=... optional (for multiple Surefire executions)

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.11
  • Fix Version/s: 2.12
  • Component/s: Maven Surefire Plugin
  • Labels:
    None
  • Complexity:
    Intermediate
  • Number of attachments :
    1

Description

Let's have a single module with multiple Surefire executions (e.g. with different Arquillian configs)
Tests are divided to run in either one, using <includes> and <excludes>.

Then, if you use -Dtest=..., the specified test(s) is run twice - once for each execution (and usually fails in one of them in our scenario).

My suggestion is to introduce a Surefire config property which would make this behavior optional:

<configuration>
  <ignoreIncludesOnSingleTest>false</ignoreIncludesOnSingleTest>
</configuration>

This would cause Surefire to run the intersection of the two sets -
one created by the mask from -Dtest=...,
second created by the includes and excludes of the respective execution.

Current description from http://maven.apache.org/plugins/maven-surefire-plugin/test-mojo.html :

Specify this parameter to run individual tests by file name, overriding the includes/excludes parameters. Each pattern you specify here will be used to create an include pattern formatted like **/$

Unknown macro: {test}

.java, so you can just type "-Dtest=MyTest" to run a single test called "foo/MyTest.java".
This parameter overrides the includes/excludes parameters, and the TestNG suiteXmlFiles parameter.

Issue Links

is related to

Improvement - An improvement or enhancement to an existing feature or task. SUREFIRE-778 failIfNoTests=false should not be required when using -Dtest=MyTestClass in multi-module reactor builds

  • Major - Major loss of function.
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
John Casey added a comment -

This is a minimal test case that illustrates the problem.

Follow instructions in README.txt.

Show
John Casey added a comment - This is a minimal test case that illustrates the problem. Follow instructions in README.txt.
Hide
Permalink
Nicolas Liochon added a comment -

Note that you have the same issues when playing with categories.

Show
Nicolas Liochon added a comment - Note that you have the same issues when playing with categories.
Hide
Permalink
Kristian Rosenvold added a comment -

It seems to me like this should be fixed in relation to 778. If include/exclude filtering is active, -DfailIfNoTests should probably be false. Conversely it should probably fail if filtering is off.

I'm also not entirely happy with the property name chosen for this feature, maybe we can find some better way to identify this..

Show
Kristian Rosenvold added a comment - It seems to me like this should be fixed in relation to 778. If include/exclude filtering is active, -DfailIfNoTests should probably be false. Conversely it should probably fail if filtering is off. I'm also not entirely happy with the property name chosen for this feature, maybe we can find some better way to identify this..
Hide
Permalink
John Casey added a comment -

I've been looking at the code, and it seems like the most elegant solution might be to create a new implementation of ScannerFilter that would wrap any provider-specific filters in the event -Dtest= is specified on the command line. Then, rather than including the -Dtest= classes in the includes for the scanner, they would be managed via a separate property list, and populate the new filter. When the DirectoryScanner filters the classes it finds based on includes/excludes, the new ScannerFilter would first make sure (a) the tests list is empty, or (b) the class matches something in the tests list...if it did match, it would pass the class on to the provider-specific filter for ultimate acceptance.

This requires three basic changes:

1. Alter the handling of -Dtest= so the enumerated tests are serialized in a different list of properties (not in the includes list)
2. Read the new property list for -Dtest=, and use it to construct a new ScannerFilter that wraps the provider-specific one. If -Dtest= is specified, only allow classes that match that property list (subject to the provider-specific filter's acceptance).
3. Provide a flag to turn on/off this test-vs-includes separation. When off, the -Dtest= properties would be used as includes, and everything acts as it does today. Otherwise, steps 1 and 2 are executed.

As far as the flag used to turn on this option, I'm less sure...maybe something like <filterCLITests>true|false</filterCLITests>?

Show
John Casey added a comment - I've been looking at the code, and it seems like the most elegant solution might be to create a new implementation of ScannerFilter that would wrap any provider-specific filters in the event -Dtest= is specified on the command line. Then, rather than including the -Dtest= classes in the includes for the scanner, they would be managed via a separate property list, and populate the new filter. When the DirectoryScanner filters the classes it finds based on includes/excludes, the new ScannerFilter would first make sure (a) the tests list is empty, or (b) the class matches something in the tests list...if it did match, it would pass the class on to the provider-specific filter for ultimate acceptance. This requires three basic changes: 1. Alter the handling of -Dtest= so the enumerated tests are serialized in a different list of properties (not in the includes list) 2. Read the new property list for -Dtest=, and use it to construct a new ScannerFilter that wraps the provider-specific one. If -Dtest= is specified, only allow classes that match that property list (subject to the provider-specific filter's acceptance). 3. Provide a flag to turn on/off this test-vs-includes separation. When off, the -Dtest= properties would be used as includes, and everything acts as it does today. Otherwise, steps 1 and 2 are executed. As far as the flag used to turn on this option, I'm less sure...maybe something like <filterCLITests>true|false</filterCLITests>?
Hide
Permalink
John Casey added a comment -

Oh, also, following up on SUREFIRE-778 ...if -DfilterCLITests=true and -Dtest= is supplied, then failIfNoTests should be set to false automatically (unless specifically overridden?)

Show
John Casey added a comment - Oh, also, following up on SUREFIRE-778 ...if -DfilterCLITests=true and -Dtest= is supplied, then failIfNoTests should be set to false automatically (unless specifically overridden?)
Hide
Permalink
Kristian Rosenvold added a comment -

Sounds like a good plan. I'll take a look at the code wrt item 2 since it's not at the top of my head. There should be unit tests to expand covering most of these functionality changes. The integration test coverage is excellent (there is a -Pparallel profile for the IT's btw), and at least one IT should be added; if you don't break any IT's your change is good

The thing is, if I understand this correctly, it's not just "ignoreIncludesOnSingleTest", because setting it to false also implies setting failIfNoTests=false, so maybe something like "testOnlyIfApplicable" that defaults to false. Bah.

Show
Kristian Rosenvold added a comment - Sounds like a good plan. I'll take a look at the code wrt item 2 since it's not at the top of my head. There should be unit tests to expand covering most of these functionality changes. The integration test coverage is excellent (there is a -Pparallel profile for the IT's btw), and at least one IT should be added; if you don't break any IT's your change is good The thing is, if I understand this correctly, it's not just "ignoreIncludesOnSingleTest", because setting it to false also implies setting failIfNoTests=false, so maybe something like "testOnlyIfApplicable" that defaults to false. Bah.
Hide
Permalink
John Casey added a comment -

Created failIfNoSpecifiedTests parameter (-Dsurefire.failIfNoSpecifiedTests and -Dit.failIfNoSpecifiedTests when used from the CLI), which will determine whether test-execution blocks that don't contain one of the tests specified on the command line should fail the build or not. Default is to fail the build.

Also, in cases where there are multiple test execution blocks, to avoid running a specified test in the wrong block, the existing includes/excludes are now honored...the specified tests now act as a refining filter on these includes/excludes. This means that in cases where there are multiple test-execution blocks, you cannot run a test that wouldn't ordinarily be run by just using -Dtest=... any more.

In cases where there is only a single test-execution block, the specified tests should override the includes as before.

I also added three new integration tests to verify all of this behavior.

Show
John Casey added a comment - Created failIfNoSpecifiedTests parameter (-Dsurefire.failIfNoSpecifiedTests and -Dit.failIfNoSpecifiedTests when used from the CLI), which will determine whether test-execution blocks that don't contain one of the tests specified on the command line should fail the build or not. Default is to fail the build. Also, in cases where there are multiple test execution blocks, to avoid running a specified test in the wrong block, the existing includes/excludes are now honored...the specified tests now act as a refining filter on these includes/excludes. This means that in cases where there are multiple test-execution blocks, you cannot run a test that wouldn't ordinarily be run by just using -Dtest=... any more. In cases where there is only a single test-execution block, the specified tests should override the includes as before. I also added three new integration tests to verify all of this behavior.

People

  • Assignee:
    John Casey
    Reporter:
    Ondrej Zizka
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: