You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/03/25 07:59:33 UTC
[GitHub] [maven-surefire] slawekjaranowski opened a new pull request #499: Remove TestListResolver#optionallyWildcardFilter
slawekjaranowski opened a new pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499
Following this checklist to help us incorporate your
contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/SUREFIRE) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `SUREFIRE-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean install` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the integration tests successfully (`mvn -Prun-its clean install`).
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835299097
##########
File path: surefire-providers/common-junit48/src/test/java/org/apache/maven/surefire/common/junit48/FilterFactoryTest.java
##########
@@ -933,8 +933,7 @@ public void testWithJUnitCoreFirstClassAndSingleMethod()
@Test
public void testShouldRunSuite()
{
- TestListResolver filter = new TestListResolver( "Su?te" );
- filter = TestListResolver.optionallyWildcardFilter( filter );
+ TestListResolver filter = new TestListResolver( "*.class" );
Review comment:
please try:
```
TestListResolver filter = new TestListResolver( "xxxx" );
filter = TestListResolver.optionallyWildcardFilter( filter );
....
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835311883
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
Wildcard class and wildcard method returns tru from `hasMethodPatterns()`? If it is any wildcard, even this special case, the filter should be returned as null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#issuecomment-1078754410
@Tibor17
- build pass
- code is more readable for me
- after review issue will be created
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r836228298
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
@slawekjaranowski
I know what you mean but the problem is that the expression
`methodFilter.isWildcard() != methodFilter.hasMethodPatterns()`
talks about implementation behavior.
The whole problem is that we adapt the tests for implementation behavior to pass because we trust the current behavior more than the tests.
Maybe the wildcard was not very properly implemented. Maybe it was, I do not remember, but if we remove some test or method we should compensate it with such strong tests and assertions where we would prove your logical expression. If `hasMethodPatterns()` is enough, why do not you write unit test asserting `isWildcard() != hasMethodPatterns()` for wildcards. That's the reason why I started the [discussion](https://github.com/apache/maven-surefire/pull/499/files/d9d914eb773b95e5a935994a6a033b4870187237#r835288021).
The filter is useless and should be null or should not be used for wildcards and empty method patterns.
Why should we slow down the JUnit or TestNG with wildcard filters or why should we let the filtering mechanism to open some unknown bug?
```
tlr = new TestListResolver( "*#*" );
assertFalse( tlr.isWildcard() );
assertTrue( tlr.hasMethodPatterns() );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835311883
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
Wildcard class and wildcard method returns `true` from `hasMethodPatterns()`? If it is any wildcard, even this special case, the filter should be returned as null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835285790
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -267,13 +270,16 @@ public void testShouldRunAny()
public void testClassFilter()
{
TestListResolver resolver = new TestListResolver( "#test" );
- assertTrue( resolver.shouldRun( "pkg/MyTest.class", null ) );
+ assertTrue( resolver.shouldRun( "pkg/MyTest.class", "test" ) );
Review comment:
Don't change the assertions.
It makes whole PR not trusty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r836219035
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -401,19 +406,6 @@ public void testInclusiveWithDefaultExclusivePattern()
assertTrue( runnable );
}
- public void testWildcard()
Review comment:
@slawekjaranowski
This test would be too simple if we have rewrite `optionallyWildcardFilter` to `**/*.class`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835403846
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
I removed `isWildcard` also ...
Test for master code:
```java
tlr = new TestListResolver( "*.class" );
assertTrue( tlr.isWildcard() );
assertFalse( tlr.hasMethodPatterns() );
tlr = new TestListResolver( "*#*" );
assertFalse( tlr.isWildcard() );
assertTrue( tlr.hasMethodPatterns() );
tlr = new TestListResolver( "**/*#*" );
assertFalse( tlr.isWildcard() );
assertTrue( tlr.hasMethodPatterns() );
```
`isWildicard()` return true only for `*.class`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835288746
##########
File path: surefire-providers/common-junit48/src/test/java/org/apache/maven/surefire/common/junit48/FilterFactoryTest.java
##########
@@ -933,8 +933,7 @@ public void testWithJUnitCoreFirstClassAndSingleMethod()
@Test
public void testShouldRunSuite()
{
- TestListResolver filter = new TestListResolver( "Su?te" );
- filter = TestListResolver.optionallyWildcardFilter( filter );
+ TestListResolver filter = new TestListResolver( "*.class" );
Review comment:
Don't change the assertions.
It makes whole PR not trusty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835311883
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
Wildcard class and wildcard method returns `true` from `hasMethodPatterns()`? If it is any wildcard, even this special case, the filter should be returned as null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835288021
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -401,19 +406,6 @@ public void testInclusiveWithDefaultExclusivePattern()
assertTrue( runnable );
}
- public void testWildcard()
Review comment:
This use case is still valid and the test should not be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835285790
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -267,13 +270,16 @@ public void testShouldRunAny()
public void testClassFilter()
{
TestListResolver resolver = new TestListResolver( "#test" );
- assertTrue( resolver.shouldRun( "pkg/MyTest.class", null ) );
+ assertTrue( resolver.shouldRun( "pkg/MyTest.class", "test" ) );
Review comment:
Don't change the assertions.
It make whole PR not trusty.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835306825
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/testset/TestListResolver.java
##########
@@ -135,27 +131,6 @@ public boolean hasMethodPatterns()
return hasIncludedMethodPatterns() || hasExcludedMethodPatterns();
}
- /**
- *
- * @param resolver filter possibly having method patterns
- * @return {@code resolver} if {@link TestListResolver#hasMethodPatterns() resolver.hasMethodPatterns()}
- * returns {@code true}; Otherwise wildcard filter {@code *.class} is returned.
- */
- public static TestListResolver optionallyWildcardFilter( TestListResolver resolver )
- {
- return resolver.hasMethodPatterns() ? resolver : WILDCARD;
- }
-
- public static TestListResolver getEmptyTestListResolver()
- {
- return EMPTY;
- }
-
- public final boolean isWildcard()
Review comment:
This is still a valid case.
If TLR is empty (nothing) or a wildcard (everything), the provider would ignore TLR. But both corner cases may happen in the config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835309076
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
We are missing `methodFilter.isWildcard()` in this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835322465
##########
File path: surefire-providers/common-junit48/src/test/java/org/apache/maven/surefire/common/junit48/FilterFactoryTest.java
##########
@@ -933,8 +933,7 @@ public void testWithJUnitCoreFirstClassAndSingleMethod()
@Test
public void testShouldRunSuite()
{
- TestListResolver filter = new TestListResolver( "Su?te" );
- filter = TestListResolver.optionallyWildcardFilter( filter );
+ TestListResolver filter = new TestListResolver( "*.class" );
Review comment:
yes, it's the same, keep this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835409879
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
In expresion:
``` java
TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
return methodFilter.isEmpty() || methodFilter.isWildcard()
? null : new TestResolverFilter( methodFilter );
```
`methodFilter.isEmpty()` always return false
`methodFilter.isWildcard() != methodFilter.hasMethodPatterns()` is always true
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835414115
##########
File path: surefire-api/src/main/java/org/apache/maven/surefire/api/testset/TestListResolver.java
##########
@@ -135,27 +131,6 @@ public boolean hasMethodPatterns()
return hasIncludedMethodPatterns() || hasExcludedMethodPatterns();
}
- /**
- *
- * @param resolver filter possibly having method patterns
- * @return {@code resolver} if {@link TestListResolver#hasMethodPatterns() resolver.hasMethodPatterns()}
- * returns {@code true}; Otherwise wildcard filter {@code *.class} is returned.
- */
- public static TestListResolver optionallyWildcardFilter( TestListResolver resolver )
- {
- return resolver.hasMethodPatterns() ? resolver : WILDCARD;
- }
-
- public static TestListResolver getEmptyTestListResolver()
- {
- return EMPTY;
- }
-
- public final boolean isWildcard()
Review comment:
only one case is true for `isWildcard`:
```java
tlr = new TestListResolver( "*.class" );
assertTrue( tlr.isWildcard() );
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835294431
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -267,13 +270,16 @@ public void testShouldRunAny()
public void testClassFilter()
{
TestListResolver resolver = new TestListResolver( "#test" );
- assertTrue( resolver.shouldRun( "pkg/MyTest.class", null ) );
+ assertTrue( resolver.shouldRun( "pkg/MyTest.class", "test" ) );
Review comment:
try:
```
resolver = new TestListResolver( "#abc" );
assertTrue( resolver.shouldRun( "pkg/MyTest.class", null ) );
resolver = new TestListResolver( "#test" );
assertTrue( resolver.shouldRun( "pkg/Abc.class", null ) );
```
what we verify here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835318041
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -267,13 +270,16 @@ public void testShouldRunAny()
public void testClassFilter()
{
TestListResolver resolver = new TestListResolver( "#test" );
- assertTrue( resolver.shouldRun( "pkg/MyTest.class", null ) );
+ assertTrue( resolver.shouldRun( "pkg/MyTest.class", "test" ) );
Review comment:
It means that whatever class should run.
This verifies ParentRunner in JUnit4.
If you type `resolver.shouldRun( "pkg/Abc.class", "otherMethod" )`, it returns false because it verifies the child of ParentRunner. Methods do not match * = Abc but abc !=otherMethod
If shouldRun has null method then we compare classes => ParentRunner, and in this case * matches Abc => true
If shouldRun has non-null method, we evaluate test method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] slawekjaranowski commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
slawekjaranowski commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835300627
##########
File path: surefire-api/src/test/java/org/apache/maven/surefire/api/testset/TestListResolverTest.java
##########
@@ -401,19 +406,6 @@ public void testInclusiveWithDefaultExclusivePattern()
assertTrue( runnable );
}
- public void testWildcard()
Review comment:
but I removed ... `#optionallyWildcardFilter` so what I should check?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-surefire] Tibor17 commented on a change in pull request #499: Remove TestListResolver#optionallyWildcardFilter
Posted by GitBox <gi...@apache.org>.
Tibor17 commented on a change in pull request #499:
URL: https://github.com/apache/maven-surefire/pull/499#discussion_r835311883
##########
File path: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java
##########
@@ -432,7 +430,6 @@ private static boolean hasFilteredOutAllChildren( Description description )
private Filter createMethodFilter()
{
- TestListResolver methodFilter = optionallyWildcardFilter( testResolver );
- return methodFilter.isEmpty() || methodFilter.isWildcard() ? null : new TestResolverFilter( methodFilter );
+ return testResolver != null && testResolver.hasMethodPatterns() ? new TestResolverFilter( testResolver ) : null;
Review comment:
Wildcard class and wildcard method (i.e. *#*, or **/*#*) returns `true` from `hasMethodPatterns()`? If it is any wildcard, even this special case, the filter should be returned as null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org