[SUREFIRE-2037] Allow for surefire-junit-platform's TestMethodFilter … #487
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,47 @@ | |
| </includes> | ||
| </configuration> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-jar-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>api-jar</id> | ||
| <phase>package</phase> | ||
| <goals> | ||
| <goal>jar</goal> | ||
| </goals> | ||
| <configuration> | ||
| <classifier>api</classifier> | ||
|
There was a problem hiding this comment. Wasn't sure whether to classify this JAR with In any case, this is simply a slim version of the JAR for service providers who otherwise do not depend on There was a problem hiding this comment. This looks like a hack. Why you have introduced Apache SPI and you change the default behavior this way? JUnit5 SPI do not exist with same purpose? The plugin has configuration of class/methods. If somebody changes this in the SPI, then what you be valid? Configuration or SPI? Pls describe very properly what you are aiming for in the JIRA in details. There was a problem hiding this comment. There are other two SPI modules There was a problem hiding this comment.
Absolutely. Here is my implementation for ArchUnit's It solves the problem of adding support for applying the same filtering logic Surefire now applies to methods, to other test sources ( In case you're unfamiliar with ArchUnit, here's what an ArchUnit test might look like: As you can see, test cases are represented by fields, and not methods. This is also why the default Surefire
Yes, of course it exists, and that's exactly what ArchUnit does under the hood. There's no way to influence Surefire filtering logic from inside a JUnit extension, though. I've already verified providing this implementation of the new SPI in ArchUnit solves the linked ArchUnit issue for Surefire. I'm now about to suggest similar extensibility to other build tools. There was a problem hiding this comment. Also, if I understand correctly, here:
You're asking what happens if someone overwrites the service to something else than Basically, there's no 'changing the SPI'. Thus, for those users who do not extend this SPI in any way, the behavior of Surefire will be consistent with the behavior before this PR.
Yes, and unfortunately, they do not fit my use case. I could implement a custom |
||
| <includes> | ||
| <include>org/apache/maven/surefire/junitplatform/TestSelectorFactory.class</include> | ||
| </includes> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>build-helper-maven-plugin</artifactId> | ||
| <version>3.2.0</version> | ||
| <executions> | ||
| <execution> | ||
| <id>api-jar</id> | ||
| <goals> | ||
| <goal>attach-artifact</goal> | ||
| </goals> | ||
| <configuration> | ||
| <artifacts> | ||
| <artifact> | ||
| <classifier>api</classifier> | ||
| <type>jar</type> | ||
| <file>${project.build.directory}/${project.artifactId}-${project.version}-api.jar</file> | ||
| </artifact> | ||
| </artifacts> | ||
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package org.apache.maven.surefire.junitplatform; | ||
|
|
||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| import org.junit.platform.engine.TestSource; | ||
| import org.junit.platform.engine.support.descriptor.MethodSource; | ||
|
|
||
| /** | ||
| * The default implementation for {@link TestSelectorFactory}, recognizing instances of {@link MethodSource} | ||
| */ | ||
| public class MethodSelectorFactory implements TestSelectorFactory | ||
| { | ||
| @Override | ||
| public boolean supports( TestSource source ) | ||
| { | ||
| return source instanceof MethodSource; | ||
| } | ||
|
|
||
| @Override | ||
| public String getContainerName( TestSource source ) | ||
| { | ||
| return ( ( MethodSource ) source ).getClassName(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getSelectorName( TestSource source ) | ||
| { | ||
| return ( ( MethodSource ) source ).getMethodName(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| package org.apache.maven.surefire.junitplatform; | ||
|
|
||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
|
|
||
| import org.apache.maven.surefire.api.testset.TestListResolver; | ||
| import org.junit.platform.engine.TestSource; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * A test selector factory used in combination with a {@link org.apache.maven.surefire.api.testset.TestListResolver} | ||
| * to determine whether a given {@link org.junit.platform.engine.TestSource} should be considered for running. | ||
| * | ||
| * <br><br><p>This is a service provider interface; clients may provide their own implementations | ||
| * that will be applied along the default {@link MethodSelectorFactory}</p> | ||
| */ | ||
| public interface TestSelectorFactory | ||
| { | ||
|
|
||
| boolean supports( TestSource source ); | ||
|
|
||
| String getContainerName( TestSource source ); | ||
|
|
||
| String getSelectorName( TestSource source ); | ||
|
|
||
| default boolean isClassContainer() | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| default TestSelector createSelector( TestSource source ) | ||
| { | ||
| String containerName = getContainerName( source ); | ||
| return new TestSelector( | ||
| isClassContainer() ? TestListResolver.toClassFileName( containerName ) : containerName, | ||
| getSelectorName( source ) ); | ||
| } | ||
|
|
||
| /** | ||
| * Represents a single test case selector | ||
| * (e.g. a fully-qualified class name + test-annotated method name) | ||
| */ | ||
| class TestSelector | ||
| { | ||
| private final String containerName; | ||
| private final String selectorName; | ||
|
|
||
| public TestSelector( String containerName, String selectorName ) | ||
| { | ||
| this.containerName = containerName; | ||
| this.selectorName = selectorName; | ||
| } | ||
|
|
||
| public String getContainerName() | ||
| { | ||
| return containerName; | ||
| } | ||
|
|
||
| public String getSelectorName() | ||
| { | ||
| return selectorName; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals( Object o ) | ||
| { | ||
| if ( this == o ) | ||
| { | ||
| return true; | ||
| } | ||
| if ( o == null || getClass() != o.getClass() ) | ||
| { | ||
| return false; | ||
| } | ||
| TestSelector that = ( TestSelector ) o; | ||
| return containerName.equals( that.containerName ) && Objects.equals( selectorName, that.selectorName ); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() | ||
| { | ||
| return Objects.hash( containerName, selectorName ); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| org.apache.maven.surefire.junitplatform.MethodSelectorFactory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't change this POM, it has nothing to do with what you are aiming for. Rather do not make any additional refactoring because it is very hard to separate reasonable and non-reasonable changes. I guess you wanted to make some specific improvement in JUnit5 provider. Do not press CTRL+L because there is again unnecessary changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I follow.
This was an intentional change, otherwise Rat would expect a license inside META-INF/services/org.apache.maven.surefire.junitplatform.TestSelectorFactory
Should I add an entry here for that specific file instead?