[GitHub] maven-surefire issue #179: [SUREFIRE-1479] Force UNIX Standard mode for ps c...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/179 @jbliznak Thx for the fix. We have 5 pending branches to accept with conflicts in the same Java file. So we will take them first and rebase this one afterwards. I have assigned this issue to the nearest Version. Currently we are fixing Jenkins CI issues where long Windows paths make harder work for us to distinguish between serious issues and trivial ones. So we have to complete CI setup. ---
[GitHub] maven-surefire issue #176: [SUREFIRE-1495] Encoding of TXT report file shoul...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/176 @eolivelli done. I did git commit amend so you have to clone the repo again. ---
[GitHub] maven-surefire issue #177: [SUREFIRE-1498] Surefire prints own logs "Couldn'...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/177 Maybe not necessary to test it again because I downloaded logs from our Jenkins ans I see the IT JUnit48TestCategoriesIT_testBadCategoryForkAlways which has the Warning message in log.txt and not in the *.dump stream. [INFO] [INFO] --- [INFO] T E S T S [INFO] --- [WARNING] Couldn't load group class 'BadCategory' in Surefire|Failsafe plugin. The group class is ignored! [WARNING] Couldn't load group class 'BadCategory' in Surefire|Failsafe plugin. The group class is ignored! [WARNING] Couldn't load group class 'BadCategory' in Surefire|Failsafe plugin. The group class is ignored! [INFO] [INFO] Results: [INFO] [INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0 ---
[GitHub] maven-surefire issue #177: [SUREFIRE-1498] Surefire prints own logs "Couldn'...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/177 @eolivelli Pls check it our with your code. The log should not appear in the dump file. ---
[GitHub] maven-surefire issue #177: [SUREFIRE-1498] Surefire prints own logs "Couldn'...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/177 @eolivelli If you know how to do it in CI, just tell me. You can see CI Mulibranch Pipeline in https://builds.apache.org/job/maven-wip/job/maven-surefire/ ---
[GitHub] maven-surefire pull request #178: SUREFIRE-1422
GitHub user Tibor17 opened a pull request: https://github.com/apache/maven-surefire/pull/178 SUREFIRE-1422 Related to JIRA [SUREFIRE-1422](https://issues.apache.org/jira/browse/SUREFIRE-1422). You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/maven-surefire SUREFIRE-1422 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven-surefire/pull/178.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #178 commit 27f68718b7cd7d73ae7488fb8b971c3d74becdef Author: Tibor17 <tibordigana@...> Date: 2018-03-10T16:10:35Z SUREFIRE-1422 ---
[GitHub] maven-surefire pull request #177: [SUREFIRE-1498] Surefire prints own logs "...
GitHub user Tibor17 opened a pull request: https://github.com/apache/maven-surefire/pull/177 [SUREFIRE-1498] Surefire prints own logs "Couldn't load group class" to native stream. Related to JIRA [SUREFIRE-1498](https://issues.apache.org/jira/browse/SUREFIRE-1498). You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/maven-surefire SUREFIRE-1498 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven-surefire/pull/177.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #177 commit af7ae9ef933873a038b2817d6430859f2a5bd1c7 Author: Tibor17 <tibordigana@...> Date: 2018-03-09T04:50:01Z [SUREFIRE-1498] Surefire prints own logs "Couldn't load group class" to native stream. ---
[GitHub] maven-surefire pull request #176: [SUREFIRE-1495] Encoding of TXT report fil...
GitHub user Tibor17 opened a pull request:
https://github.com/apache/maven-surefire/pull/176
[SUREFIRE-1495] Encoding of TXT report file should be configured by
${project.reporting.outputEncoding} and MOJO parameter encoding
Related to JIRA
[SUREFIRE-1495](https://issues.apache.org/jira/browse/SUREFIRE-1495).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/maven-surefire SUREFIRE-1495
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/maven-surefire/pull/176.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #176
commit c496ebf3b4518c25787d5eac9f54b2a220a59447
Author: Tibor17 <tibordigana@...>
Date: 2018-03-08T07:19:02Z
[SUREFIRE-1495] Encoding of TXT report file should be configured by
${project.reporting.outputEncoding} and MOJO parameter encoding
---
[GitHub] maven-surefire pull request #175: [SUREFIRE-1490] Change header of the Fails...
GitHub user Tibor17 opened a pull request: https://github.com/apache/maven-surefire/pull/175 [SUREFIRE-1490] Change header of the Failsafe Report Related to JIRA [SUREFIRE-1490](https://issues.apache.org/jira/browse/SUREFIRE-1490). You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/maven-surefire SUREFIRE-1490 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven-surefire/pull/175.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #175 commit 6174be76f52ffc2873a18a5215f60767843d7479 Author: Tibor17 <tibordigana@...> Date: 2018-03-07T01:43:37Z [SUREFIRE-1490] Change header of the Failsafe Report ---
[GitHub] maven-surefire issue #163: Surefire-report
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/163 @shafiullas Please close this PR: https://github.com/apache/maven-surefire/pull/163 Thx ---
[GitHub] maven-surefire issue #174: https://issues.apache.org/jira/browse/SUREFIRE-14...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/174 @awallgren You can close this PR. There is already a branch with dedicated fix: https://git-wip-us.apache.org/repos/asf?p=maven-surefire.git;a=shortlog;h=refs/heads/SUREFIRE-1475 Thx for contributing! ---
[GitHub] maven-surefire issue #174: https://issues.apache.org/jira/browse/SUREFIRE-14...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/174 @awallgren Are you here? ---
[GitHub] maven-surefire issue #174: https://issues.apache.org/jira/browse/SUREFIRE-14...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/174 Of course you should push only single commit in PR. `git commit --amend` and the title must be `[SUREFIRE-1475] ...here is title as in Jira...` ---
[GitHub] maven-surefire issue #174: https://issues.apache.org/jira/browse/SUREFIRE-14...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/174 First of all do it similar like we do in *NIX. Resolve `%SystemRoot%\System32\Wbem\wmic.exe` in Java and check that the file exists. If it exists use it, otherwise do it as we did before. ---
[GitHub] maven-surefire issue #173: [SUREFIRE-1004] Support full gavtc format for dep...
Github user Tibor17 commented on the issue:
https://github.com/apache/maven-surefire/pull/173
Thx for your work. I will get back to you soon. Right now I am working on
branch SUREFIRE-1463 which contains few fixes. I plan to finish this,
including build script, and then I would push my changes. I will make a
code review in this PR as I do in every other before accepting. Please wait
for me meanwhile.
On Sat, Feb 10, 2018 at 1:42 AM, Bindul Bhowmik <notificati...@github.com>
wrote:
> @Tibor17 <https://github.com/tibor17>
>
> I have reviewed the Maven Coordinates documentation you mentioned, and I
> can switch the order of elements in the parameter easily. However, I think
> I would disagree with requiring the version to be last element in the
> coordinates in this use case. As the functionality stands, the
> dependenciesToScan configuration does not add additional dependencies to
> the test scope of the project, it filters dependencies already added to
the
> test scope in the project to allow for scanning test classes to run. If
> someone wants to add say a classfier or a type/packaging, requiring them
> them to also mention the version of the dependency would just make
> maintainers life harder by having another location to keep the version of
> the dependency in sync.
>
> As such I propose, we keep the version as optional and support the
> following variations of dependencies to scan:
>
>- groupId:artifactId
>- groupId:artifactId:packaging/type
>- groupId:artifactId:packaging/type::version
>- groupId:artifactId:packaging/type:classifier
>- groupId:artifactId::classifier
>- groupId:artifactId::classifier:version
>- groupId:artifactId:packaging/type:classifier:version
>
> It still maintains the same order of elements as the Maven Coordinates
> documentation in the POM, just makes the version not required to be the
> last element, or rather makes version optional.
>
> Thoughts?
>
> â
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
>
<https://github.com/apache/maven-surefire/pull/173#issuecomment-364609503>,
> or mute the thread
>
<https://github.com/notifications/unsubscribe-auth/AA_yR2UI0i46PG43W3OWYyJ0NwaY_X9Pks5tTOYHgaJpZM4RcCcQ>
> .
>
--
Cheers
Tibor
---
[GitHub] maven-surefire issue #174: https://issues.apache.org/jira/browse/SUREFIRE-14...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/174 Logging errors can be added to the dump file, but right now please add the Microsoft link, as you said 'yes'. ---
[GitHub] maven-surefire issue #174: https://issues.apache.org/jira/browse/SUREFIRE-14...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/174 Did we take right path here as well? Did Microsoft mention this link in their documentation? ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 Now there is only one issue to fix related to Windows paths. Need to investigate. https://builds.apache.org/job/maven-surefire-pipeline/job/SUREFIRE-1463/121 [windows-jdk9-maven3.5.x] shouldMatchSimpleClassNameAndMethodWithJavaExtWildcardPkg[0](org.apache.maven.surefire.its.TestMultipleMethodPatternsTestNGIT) On Sun, Feb 4, 2018 at 9:06 PM, Tibor Digana <tibordig...@apache.org> wrote: > The project has new build script and testing JDK 7-10 in combination of > multiple platforms. That is the main purpose of Version 2.21.0. Sorry that > it takes so long. We are discovering issues and fixing them one by one. I > think we are close to the end. > > On Fri, Feb 2, 2018 at 4:02 PM, hemachandraraok <notificati...@github.com> > wrote: > >> Sure and let me wait! >> Thank you so much for the updates!!!Cheers! >> >> â >> You are receiving this because you were mentioned. >> Reply to this email directly, view it on GitHub >> <https://github.com/apache/maven-surefire/pull/150#issuecomment-362610229>, >> or mute the thread >> <https://github.com/notifications/unsubscribe-auth/AA_yR4VCbLtVu6ypblIkJm7ZSxUbmhBGks5tQyOcgaJpZM4NoCQj> >> . >> > > ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 The project has new build script and testing JDK 7-10 in combination of multiple platforms. That is the main purpose of Version 2.21.0. Sorry that it takes so long. We are discovering issues and fixing them one by one. I think we are close to the end. On Fri, Feb 2, 2018 at 4:02 PM, hemachandraraok <notificati...@github.com> wrote: > Sure and let me wait! > Thank you so much for the updates!!!Cheers! > > â > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/apache/maven-surefire/pull/150#issuecomment-362610229>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AA_yR4VCbLtVu6ypblIkJm7ZSxUbmhBGks5tQyOcgaJpZM4NoCQj> > . > ---
[GitHub] maven-surefire issue #173: [SUREFIRE-1004] Support full gavtc format for dep...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/173 @bindul Then we have to support also other alternatives: `groupId:artifactId:version`, and `groupId:artifactId:packaging:version`. This means you have to, first of all, count `:` and call different private method/matcher for count=2, 3, 4 or 5. ---
[GitHub] maven-surefire issue #173: [SUREFIRE-1004] Support full gavtc format for dep...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/173 @binduk We have modify the maven coordinates to `groupId:artifactId:packaging:classifier:version`, see the docs: https://maven.apache.org/pom.html#Maven_Coordinates Basically, only the version would be determined last. ---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161337637
--- Diff:
maven-surefire-plugin/src/site/apt/examples/inclusion-exclusion.apt.vm ---
@@ -209,4 +209,41 @@ Inclusions and Exclusions of Tests
Multiple formats can be additionally combined.
This syntax can be used in parameters: <<>>, <<>>,
<<>>, <<>>, <<>>.
+* Tests from dependencies
+
+ By default, ${thisPlugin} will only scan for test classes to execute in
the configured <<>>. To
--- End diff --
Here you should say that the feature VTC was introduced in version 2.21.0:
`Since 2.21.0 ...`
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request: https://github.com/apache/maven-surefire/pull/173#discussion_r161336602 --- Diff: surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1004RunTestFromDependencyJarsTypeAndClassifierIT.java --- @@ -0,0 +1,57 @@ +package org.apache.maven.surefire.its.jiras; + +/* + * 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 static org.junit.Assert.*; --- End diff -- Same here. ---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161336480
--- Diff:
maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/util/DependenciesScannerTest.java
---
@@ -69,19 +78,141 @@ public void testLocateTestClasses()
assertEquals( 1, props.size() );
}
-private File writeTestFile()
+/**
+ * Test for artifact with classifier
+ */
+@Test
+public void testLocateTestClassesFromArtifactWithClassifier()
+throws Exception
+{
+File testJarFile = writeTestFile(
"DependenciesScannerTest2-1.0-tests-jdk15.jar", "org/test/TestA.class",
+ "org/test/other/TestAA.class",
"org/test/TestB.class" );
+Artifact testArtifact =
+new DefaultArtifact( "org.surefire.dependency",
"dependent-artifact2",
+ VersionRange.createFromVersion( "1.0" ),
"test", "jar", "tests-jdk15", null );
+testArtifact.setFile( testJarFile );
+
+List scanDependencies = new ArrayList();
+scanDependencies.add(
"org.surefire.dependency:dependent-artifact2:::tests-jdk15" );
+
+List include = new ArrayList();
+include.add( "**/*A.java" );
+List exclude = new ArrayList();
+
+DependencyScanner scanner =
+new DependencyScanner( DependencyScanner.filter(
Collections.singletonList( testArtifact ),
+
scanDependencies ),
+ new TestListResolver( include, exclude
) );
+
+ScanResult classNames = scanner.scan();
+assertNotNull( classNames );
+assertEquals( 2, classNames.size() );
+
+Map<String, String> props = new HashMap<String, String>();
+classNames.writeTo( props );
+assertEquals( 2, props.size() );
+}
+
+/**
+ * Test with type when two artifacts are present, should only find the
class in jar with correct type
+ */
+@Test
+public void testLocateTestClassesFromMultipleArtifactsWithType()
+throws Exception
+{
+File jarFile =
+writeTestFile( "DependenciesScannerTest3-1.0.jar",
"org/test/ClassA.class", "org/test/ClassB.class" );
+Artifact mainArtifact = new DefaultArtifact(
"org.surefire.dependency", "dependent-artifact3",
+
VersionRange.createFromVersion( "1.0" ), "test", "jar", null,
+ new
DefaultArtifactHandler() );
+mainArtifact.setFile( jarFile );
+
+File testJarFile =
+writeTestFile( "DependenciesScannerTest3-1.0-tests.jar",
"org/test/TestA.class", "org/test/TestB.class" );
+Artifact testArtifact = new DefaultArtifact(
"org.surefire.dependency", "dependent-artifact3",
+
VersionRange.createFromVersion( "1.0" ), "test", "test-jar", null,
+ new
DefaultArtifactHandler() );
+testArtifact.setFile( testJarFile );
+
+List scanDependencies = new ArrayList();
+scanDependencies.add(
"org.surefire.dependency:dependent-artifact3::test-jar" );
+
+List include = new ArrayList();
+include.add( "**/*A.java" );
+List exclude = new ArrayList();
+
+DependencyScanner scanner =
+new DependencyScanner( DependencyScanner.filter(
Arrays.asList( mainArtifact, testArtifact ),
--- End diff --
Try to make the constructor call more compact using e.g. static imports or
extracting to another local variables.
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request: https://github.com/apache/maven-surefire/pull/173#discussion_r161336057 --- Diff: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java --- @@ -732,7 +732,8 @@ * The child elements of this element must be dependency elements, and the * contents of each of these elements must be a string which follows the format: * - * groupId:artifactId. For example: org.acme:project-a. + * groupId:artifactId[:version[:type[:classifier]]]. For example: org.acme:project-a, + * org.acme:project-a::test-jar, org.acme:project-a:::tests-jdk15 --- End diff -- Did the previous pattern `groupId:artifactId` support `*` wildcard. Does your algorithm support `*`? I know that other plugins usually support it with `*`. If both, empty character between :: and `*`, would work, even better. ---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request: https://github.com/apache/maven-surefire/pull/173#discussion_r161335367 --- Diff: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/util/DependenciesScannerTest.java --- @@ -19,12 +19,17 @@ * under the License. */ -import junit.framework.TestCase; +import static org.junit.Assert.*; --- End diff -- Not sure if checkstyle will break the build because of using star `*` in this import. ---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161334621
--- Diff:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/util/DependencyScanner.java
---
@@ -111,19 +112,46 @@ private static void scanArtifact( File artifact,
TestFilter<String, String> filt
{
for ( String groups : groupArtifactIds )
{
+// groupId:artifactId[:version[:type[:classifier]]]
String[] groupArtifact = groups.split( ":" );
-if ( groupArtifact.length != 2 )
+if ( groupArtifact.length < 2 || groupArtifact.length > 5 )
{
throw new IllegalArgumentException( "dependencyToScan
argument should be in format"
-+ " 'groupid:artifactid': " + groups );
++ "
'groupid:artifactid[:version[:type[:classifier]]]': " + groups );
}
-if ( artifact.getGroupId().matches( groupArtifact[0] )
-&& artifact.getArtifactId().matches( groupArtifact[1]
) )
+if ( artifactMatchesGavtc( artifact, groupArtifact ) )
{
matches.add( artifact.getFile() );
}
}
}
return matches;
}
+
+private static boolean artifactMatchesGavtc( Artifact artifact,
String[] gavtc )
+{
+boolean match = false;
+if ( artifact.getGroupId().matches( gavtc[0] ) &&
artifact.getArtifactId().matches( gavtc[1] ) )
+{
+match = true;
+// Check version
+if ( match && gavtc.length > 2 )
+{
+match = StringUtils.isBlank( gavtc[2] ) ||
artifact.getVersion().equals( gavtc[2] );
--- End diff --
Then `match = StringUtils.isBlank( gavtc[2] )` might be verbose with
calling new private static method `match = hasVersion( groups );`
private static boolean hasVersion( String... gavtc )
{
return StringUtils.isBlank( gavtc[2] );
}
Basically the same principle with T and C and the code will be verbose.
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161333957
--- Diff:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/util/DependencyScanner.java
---
@@ -111,19 +112,46 @@ private static void scanArtifact( File artifact,
TestFilter<String, String> filt
{
for ( String groups : groupArtifactIds )
{
+// groupId:artifactId[:version[:type[:classifier]]]
String[] groupArtifact = groups.split( ":" );
-if ( groupArtifact.length != 2 )
+if ( groupArtifact.length < 2 || groupArtifact.length > 5 )
{
throw new IllegalArgumentException( "dependencyToScan
argument should be in format"
-+ " 'groupid:artifactid': " + groups );
++ "
'groupid:artifactid[:version[:type[:classifier]]]': " + groups );
}
-if ( artifact.getGroupId().matches( groupArtifact[0] )
-&& artifact.getArtifactId().matches( groupArtifact[1]
) )
+if ( artifactMatchesGavtc( artifact, groupArtifact ) )
{
matches.add( artifact.getFile() );
}
}
}
return matches;
}
+
+private static boolean artifactMatchesGavtc( Artifact artifact,
String[] gavtc )
+{
+boolean match = false;
+if ( artifact.getGroupId().matches( gavtc[0] ) &&
artifact.getArtifactId().matches( gavtc[1] ) )
+{
+match = true;
+// Check version
+if ( match && gavtc.length > 2 )
+{
--- End diff --
Because I have proposed this method has only one parameter `( String groups
)` you can use our utility:
`String[] gavtc = StringUtils.split( groups, ':' );`
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161333588
--- Diff:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/util/DependencyScanner.java
---
@@ -111,19 +112,46 @@ private static void scanArtifact( File artifact,
TestFilter<String, String> filt
{
for ( String groups : groupArtifactIds )
{
+// groupId:artifactId[:version[:type[:classifier]]]
String[] groupArtifact = groups.split( ":" );
-if ( groupArtifact.length != 2 )
+if ( groupArtifact.length < 2 || groupArtifact.length > 5 )
{
throw new IllegalArgumentException( "dependencyToScan
argument should be in format"
-+ " 'groupid:artifactid': " + groups );
++ "
'groupid:artifactid[:version[:type[:classifier]]]': " + groups );
}
-if ( artifact.getGroupId().matches( groupArtifact[0] )
-&& artifact.getArtifactId().matches( groupArtifact[1]
) )
+if ( artifactMatchesGavtc( artifact, groupArtifact ) )
{
matches.add( artifact.getFile() );
}
}
}
return matches;
}
+
+private static boolean artifactMatchesGavtc( Artifact artifact,
String[] gavtc )
+{
+boolean match = false;
+if ( artifact.getGroupId().matches( gavtc[0] ) &&
artifact.getArtifactId().matches( gavtc[1] ) )
+{
+match = true;
+// Check version
+if ( match && gavtc.length > 2 )
--- End diff --
Did you mean `>=`?
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161333402
--- Diff:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/util/DependencyScanner.java
---
@@ -111,19 +112,46 @@ private static void scanArtifact( File artifact,
TestFilter<String, String> filt
{
for ( String groups : groupArtifactIds )
{
+// groupId:artifactId[:version[:type[:classifier]]]
String[] groupArtifact = groups.split( ":" );
-if ( groupArtifact.length != 2 )
+if ( groupArtifact.length < 2 || groupArtifact.length > 5 )
{
throw new IllegalArgumentException( "dependencyToScan
argument should be in format"
-+ " 'groupid:artifactid': " + groups );
++ "
'groupid:artifactid[:version[:type[:classifier]]]': " + groups );
}
-if ( artifact.getGroupId().matches( groupArtifact[0] )
-&& artifact.getArtifactId().matches( groupArtifact[1]
) )
+if ( artifactMatchesGavtc( artifact, groupArtifact ) )
{
matches.add( artifact.getFile() );
}
}
}
return matches;
}
+
+private static boolean artifactMatchesGavtc( Artifact artifact,
String[] gavtc )
+{
+boolean match = false;
+if ( artifact.getGroupId().matches( gavtc[0] ) &&
artifact.getArtifactId().matches( gavtc[1] ) )
+{
+match = true;
+// Check version
--- End diff --
I mean only these comments. For me it is quite obvious what you mean in
code and thus the comment is useless. If you want to put a meaning on the IF
statement, you can do something like this:
`if ( hasGroupAndArtifactId( groups ) )`
...
```private static boolean hasGroupAndArtifactId( String groups )
return StringUtils.countMatches( groups, ':' ) >= 2;
```
The project knows several `StringUtils` but I mean the one from library
`commons-lang3`.
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161332171
--- Diff:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/util/DependencyScanner.java
---
@@ -111,19 +112,46 @@ private static void scanArtifact( File artifact,
TestFilter<String, String> filt
{
for ( String groups : groupArtifactIds )
{
+// groupId:artifactId[:version[:type[:classifier]]]
String[] groupArtifact = groups.split( ":" );
-if ( groupArtifact.length != 2 )
+if ( groupArtifact.length < 2 || groupArtifact.length > 5 )
{
throw new IllegalArgumentException( "dependencyToScan
argument should be in format"
-+ " 'groupid:artifactid': " + groups );
++ "
'groupid:artifactid[:version[:type[:classifier]]]': " + groups );
}
-if ( artifact.getGroupId().matches( groupArtifact[0] )
-&& artifact.getArtifactId().matches( groupArtifact[1]
) )
+if ( artifactMatchesGavtc( artifact, groupArtifact ) )
{
matches.add( artifact.getFile() );
}
}
}
return matches;
}
+
+private static boolean artifactMatchesGavtc( Artifact artifact,
String[] gavtc )
--- End diff --
In my next comment I use "groups" as a parameter because the upper part has
local variable "groups". You can rename it as you like.
---
[GitHub] maven-surefire pull request #173: [SUREFIRE-1004] Support full gavtc format ...
Github user Tibor17 commented on a diff in the pull request:
https://github.com/apache/maven-surefire/pull/173#discussion_r161331717
--- Diff:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/util/DependencyScanner.java
---
@@ -111,19 +112,46 @@ private static void scanArtifact( File artifact,
TestFilter<String, String> filt
{
for ( String groups : groupArtifactIds )
{
+// groupId:artifactId[:version[:type[:classifier]]]
--- End diff --
Maybe you could add `groupId:artifactId[:version[:type[:classifier]]]` to a
regular JavaDoc on the top of Class or method and then this comment can be
removed. The users will see it in public API JavaDoc for their better
understanding.
---
[GitHub] maven-surefire issue #173: [SUREFIRE-1004] Support full gavtc format for dep...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/173 I don't want you to remove code. I will try to explain in code. ---
[GitHub] maven-surefire issue #173: [SUREFIRE-1004] Support full gavtc format for dep...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/173 The comments like this `// Check version` are not needed because it's obvious from code. You may remove them or extract that part to private static methods. It's up to you. If you change something please `git --ammend commit`. There should be single commit per PR. Good work! ---
[GitHub] maven-surefire issue #163: Surefire-report
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/163 @shafiullas Thx for contributing. The PR was modified and pushed to master. Please close this PR. ---
[GitHub] maven-surefire issue #163: Surefire-report
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/163 @shafiullas I have fixed the checkstyle in your code but I do not see text of Description in HTML. Did you try to use it in real and saw the Description on the page? ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje Thx for contributing. ---
[GitHub] maven-surefire issue #143: [SUREFIRE-1416] maven-surefire-parser: add new me...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/143 @surli Thx for contributing. Now you can close the PR. ---
[GitHub] maven-surefire issue #143: [SUREFIRE-1416] maven-surefire-parser: add new me...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/143 @surli Pls try to use it in your projects. I want to know if it is useful for you and working. Thx. ---
[GitHub] maven-surefire issue #143: [SUREFIRE-1416] maven-surefire-parser: add new me...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/143 @surli If there are no objections, I would push the branch SUREFIRE-1416 to master. Thx. ---
[GitHub] maven-surefire issue #143: [SUREFIRE-1416] maven-surefire-parser: add new me...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/143 @surli I had to change you code in new branch https://git1-us-west.apache.org/repos/asf?p=maven-surefire.git;a=shortlog;h=refs/heads/SUREFIRE-1416 Can you build the version `2.21.0-SNAPSHOT` using `mvn install -DskipTests` and let me know if the report site works for you? I want to include this fix in release in several days. ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue:
https://github.com/apache/maven-surefire/pull/150
@mpkorstanje
The build passed successfully. I will refactor little details, I will
squash our commits and then I will push it to master.
I could not find a documentation for configuring annotation
`CucumberOptions`. For instance we were using it like this
`@CucumberOptions(features = {"classpath:flake"})` in your previous commit. Why
we do not use it any more and why it was important? Do you think it is
important for the users and important for proper functionality of Surefire?
One more question I have is regarding artifacts of Cucumber. Which one is
right to use:
`cucumber-junit`
`cucumber-java8`
---
[GitHub] maven-surefire pull request #150: SUREFIRE-1372 Filter tests to be rerun by ...
Github user Tibor17 commented on a diff in the pull request: https://github.com/apache/maven-surefire/pull/150#discussion_r158589504 --- Diff: surefire-providers/surefire-junit4/src/main/java/org/apache/maven/surefire/junit4/JUnit4Provider.java --- @@ -55,7 +53,8 @@ import static java.lang.reflect.Modifier.isAbstract; import static java.lang.reflect.Modifier.isInterface; import static org.apache.maven.surefire.booter.CommandReader.getReader; -import static org.apache.maven.surefire.common.junit4.JUnit4ProviderUtil.generateFailingTests; +import static org.apache.maven.surefire.common.junit4.FilterFactory.createMatchDescriptionsFilter; --- End diff -- It seems we can move this method to `JUnit4ProviderUtil` and remove `org.apache.maven.surefire.common.junit4.FilterFactory`. ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue:
https://github.com/apache/maven-surefire/pull/150
The fast way would be to give it a try in a copy of your project apart and
run the local build
"mvn install -P run-its -Dcheckstyle.skip=true".
Even if you copied FilterFactory into JUnit47 provider and used this loop
as you have mentioned above and build it, it would be safer for you. If the
build will be finally successful, we will just move the class FilterFactory
to the module common-junit4, refactor usages and then patch the JUnit4
provider in this PR.
On Sat, Dec 23, 2017 at 9:25 PM, M.P. Korstanje <notificati...@github.com>
wrote:
> @Tibor17 <https://github.com/tibor17> that should be doable. Replacing
> the inner most loop in the JUnit4Provider.executeWithRerun with the code
> below should do it. This would require the FilterFactory to be moved to
> another common module. I can't quite tell what the the nock-on effects of
> that would be.
>
> for ( int i = 0; i < rerunFailingTestsCount &&
!failureListener.getAllFailures().isEmpty(); i++ )
> {
> Set failures = generateFailingTestDescriptions(
failureListener.getAllFailures() );
> failureListener.reset();
> FilterFactory filterFactory = new FilterFactory( testClassLoader );
> Filter failingMethodsFilter =
filterFactory.createMatchMethodDescriptionsFilter( failures );
> Request.aClass( clazz ).filterWith( failingMethodsFilter
).getRunner().run( rerunNotifier );
> }
>
> â
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
>
<https://github.com/apache/maven-surefire/pull/150#issuecomment-353746273>,
> or mute the thread
>
<https://github.com/notifications/unsubscribe-auth/AA_yR-4dWYNw6T5vwzi-xzC6CWnSZL9yks5tDWG4gaJpZM4NoCQj>
> .
>
--
Cheers
Tibor
---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje After seen the code of junit4 provider I have realized that `Filter` also exists in JUnit4. Therefore maybe using a combination of class/method and using JUnit's Request object in this kind of provider was not right step. Do you think that reworking to Filter would be doable? ---
[GitHub] maven-surefire issue #167: Speedup Standard Output if Tests
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/167 @DaGeRe Close this PR! ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje Can we continue? Do you have spare time? ---
[GitHub] maven-surefire issue #172: [SUREFIRE-1453] Allow to specify non existant cla...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/172 @eolivelli Thx for contributing. The PR was pushed to master and now it can be closed. ---
[GitHub] maven-surefire issue #167: Speedup Standard Output if Tests
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/167 @DaGeRe Close this PR on GitHub. I have made a fix. See this Jira issue: https://issues.apache.org/jira/browse/SUREFIRE-1454 ---
[GitHub] maven-surefire issue #172: [SUREFIRE-1453] Allow to specify non existant cla...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/172 Try to name the commit with brackets: ` [SUREFIRE-1453] Allow to specify non existant classes as groups` ---
[GitHub] maven-surefire issue #172: [SUREFIRE-1453] Allow to specify non existant cla...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/172 @eolivelli Thx for your work. Try to prove it with integration test. We always have to do it if it technically can be tested. ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje I saw you typed version `2.21`. It should be `2.21.0`. Pls fix this as well. Thx. ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje Now I understand the Cucumber internals. Can you write the documentation? Fix the checkstyle in your integration test and the classes `FlakeCucumberTest`. Then try to alter the behavior of the integration test with Maven profile (junit4 and junit47). We are already doing it, see `addGoal( "-P junit4" )`, `addGoal( "-P junit47" )`. This would lead to `@Parameterized` IT which exists in our code already, see `FailFastJUnitIT` as a hint. ---
[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/166 @krmahadevan Thx for contributing. Now you should close this PR. ---
[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/166 @krmahadevan There is one failing integration test: [unix] Failed tests: shouldNotRunExcludedClassesAndMethods[0] (org.apache.maven.surefire.its.TestMultipleMethodPatternsTestNGIT): wrong number of tests expected:<11> but was:<12> What is wrong? Why it failed? ---
[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/166 @krmahadevan I have created a branch `SUREFIRE-1452`, changed your commit [1] according to our needs and now the build is running [2]. [1]: https://git1-us-west.apache.org/repos/asf?p=maven-surefire.git;a=commit;h=946bf1eb8d5b56dcd10ec44c463d2f6014d01a95 [2]: https://builds.apache.org/view/M-R/view/Maven/job/maven-surefire-pipeline/job/SUREFIRE-1452/ ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje I do not have questions yet. I am just reading your comments. I would think of the code combining together with Cucumber internals and then I would have maybe some notice. ---
[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/166 @krmahadevan Do you have a spare time for Surefire? This is good work but there is only a little to do: checkstyle and Jira ticket. ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje I want to ask you few question because I want to make minimum invasive changes in the code. Why you have chosen provider `surefire-junit47` and not also the `surefire-junit4`? Why the method `generateFailingTestDescriptions` has to return `Set` and not the previous `Map<Class, Set>`? ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje Additionally, can you write a separate documentation file `cucumber.apt.vm` for Cucumber with your best practices to setup and use cucumber in terms of `maven-surefire-plugin` and `maven-failsafe-plugin` tests, annotations like `CucumberOptions`, dependencies, versions, configuration if any, and `re-run` notices? Take a notice that `maven-failsafe-plugin` has name or class *IT but `maven-surefire-plugin` has *Test. The files in folder `examples` will show you how to cope with both plugins and alter the text. The file `cucumber.apt.vm` should be located in `maven-surefire-plugin/src/site/apt/examples`. And add the following to `site.xml`: `` ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje How is `Description` constructed now by Cucumber? Does it contain the real method name or it is a scenario text? ---
[GitHub] maven-surefire pull request #150: SUREFIRE-1372 Filter tests to be rerun by ...
Github user Tibor17 commented on a diff in the pull request: https://github.com/apache/maven-surefire/pull/150#discussion_r155940679 --- Diff: surefire-integration-tests/src/test/resources/junit47-rerun-failing-tests-with-cucumber/pom.xml --- @@ -0,0 +1,79 @@ + + + +http://maven.apache.org/POM/4.0.0; + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance; + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd;> +4.0.0 + + +org.apache.maven.surefire +it-parent +1.0 + + +junit47-rerun-failing-tests-with-cucumber +jar --- End diff -- jar packaging is default one. You don't need to specify it. ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue:
https://github.com/apache/maven-surefire/pull/150
As far as I remember the filter was used in rerun feature where broken
tests were executed several more times after.
Yet let me check your branch because that's better than if you spent your
time again.
I will inform you about my findings in few days.
On Fri, Dec 8, 2017 at 5:07 PM, M.P. Korstanje <notificati...@github.com>
wrote:
> @Tibor17 <https://github.com/tibor17> It appears this branch is still up
> to date with master.
>
> Is the removal of the class the problem or the underlying fundamentals? If
> it is just the removal I can restore FailingMethodFilter.java. It went
> unused after my changes. If it is the underlying fundamentals please let
me
> know how I can help you.
>
> â
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
>
<https://github.com/apache/maven-surefire/pull/150#issuecomment-350301256>,
> or mute the thread
>
<https://github.com/notifications/unsubscribe-auth/AA_yR4f-4bf3vz9RkLIyf-k7OEB7VA4Kks5s-V6rgaJpZM4NoCQj>
> .
>
--
Cheers
Tibor
---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @mpkorstanje @ckurban @sworisbreathing I could not merge and release this PR because the class `FailingMethodFilter.java` in [1] was deleted. We have to have it back and find root cause and good cure. I may have spare time during the weekend but this troubleshooting may take several days to understand the cure. [1]: https://github.com/apache/maven-surefire/pull/150/commits/780e878a3c2d15547bc0157ebef67de793b06862 ---
[GitHub] maven-surefire issue #150: SUREFIRE-1372 Filter tests to be rerun by descrip...
Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/150 @sworisbreathing Let me go through these changes again. ---
