Uploaded image for project: 'Maven Surefire'
  1. Maven Surefire
  2. SUREFIRE-1278

TestNG tests are run with group name that ends with specified group

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.19.1
    • Fix Version/s: 2.20
    • Component/s: Maven Surefire Plugin
    • Labels:
      None
    • Environment:
      OS X El Capitan 10.11.6
      TestNG 6.9.10

      Description

      Preconditions:

      There is a simple TestNG class with two methods so that 1st method has group "group" and 2nd has group "apigroup".

      Steps to reproduce:

      Run tests with command:

      mvn clean test -Dgroups=group
      

      h4.Actual:
      Both tests are run.

      h4.Expected:
      Only method with group "group" is run.

      Demo project could be found here: https://github.com/khospodarysko/testng-groups.

      Looks like the issue is in GroupMatcherMethodSelector as looking at includeMethod it is seen that is always returns true.

      1. testng-groups.zip
        16 kB
        Kostya Hospodarysko

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khospodarysko closed the pull request at:

          https://github.com/apache/maven-surefire/pull/121

          Show
          githubbot ASF GitHub Bot added a comment - Github user khospodarysko closed the pull request at: https://github.com/apache/maven-surefire/pull/121
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khospodarysko commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          @Tibor17 No, I haven't. This is my first such patch. I'm happy it added more quality to the product!

          Show
          githubbot ASF GitHub Bot added a comment - Github user khospodarysko commented on the issue: https://github.com/apache/maven-surefire/pull/121 @Tibor17 No, I haven't. This is my first such patch. I'm happy it added more quality to the product!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Tibor17 commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          @khospodarysko CI build was successful. This should be normally closed automatically and sometimes it takes a day but you can close it as well.
          Thank you for contributing.
          Have you contributed to Maven in more previous pull-requests?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/121 @khospodarysko CI build was successful. This should be normally closed automatically and sometimes it takes a day but you can close it as well. Thank you for contributing. Have you contributed to Maven in more previous pull-requests?
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build maven-surefire #1617 (See https://builds.apache.org/job/maven-surefire/1617/)
          Fix for SUREFIRE-1278 - TestNG tests are run with group name that ends (tibor17: rev ae6337ea216b867e76e951e09951e475ed105d06)

          • (add) surefire-integration-tests/src/test/resources/surefire-1278-group-name-ending/pom.xml
          • (edit) surefire-grouper/src/main/java/org/apache/maven/surefire/group/match/SingleGroupMatcher.java
          • (add) surefire-integration-tests/src/test/resources/surefire-1278-group-name-ending/src/test/java/pkg/ATest.java
            Fix for SUREFIRE-1278 - TestNG tests are run with group name that ends (tibor17: rev bab3175e8388b675e1e56814d3b1a5aa25ae250a)
          • (add) surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1278GroupNameEndingIT.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build maven-surefire #1617 (See https://builds.apache.org/job/maven-surefire/1617/ ) Fix for SUREFIRE-1278 - TestNG tests are run with group name that ends (tibor17: rev ae6337ea216b867e76e951e09951e475ed105d06) (add) surefire-integration-tests/src/test/resources/surefire-1278-group-name-ending/pom.xml (edit) surefire-grouper/src/main/java/org/apache/maven/surefire/group/match/SingleGroupMatcher.java (add) surefire-integration-tests/src/test/resources/surefire-1278-group-name-ending/src/test/java/pkg/ATest.java Fix for SUREFIRE-1278 - TestNG tests are run with group name that ends (tibor17: rev bab3175e8388b675e1e56814d3b1a5aa25ae250a) (add) surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1278GroupNameEndingIT.java
          Hide
          tibor17 Tibor Digana added a comment -

          commit ae6337ea216b867e76e951e09951e475ed105d06
          commit bab3175e8388b675e1e56814d3b1a5aa25ae250a

          Show
          tibor17 Tibor Digana added a comment - commit ae6337ea216b867e76e951e09951e475ed105d06 commit bab3175e8388b675e1e56814d3b1a5aa25ae250a
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Tibor17 commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          LGTM.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/121 LGTM.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khospodarysko commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          Followed the analogy and added org.apache.maven.surefire.its.jiras.Surefire1278GroupNameEndingIT to src/test/java folder.
          Please let me know if something else is wrong and I'll try to fix it asap.

          Show
          githubbot ASF GitHub Bot added a comment - Github user khospodarysko commented on the issue: https://github.com/apache/maven-surefire/pull/121 Followed the analogy and added org.apache.maven.surefire.its.jiras.Surefire1278GroupNameEndingIT to src/test/java folder. Please let me know if something else is wrong and I'll try to fix it asap.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Tibor17 commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          @khospodarysko Do you have it in *.bak file or in bytecode in /target/classes?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/121 @khospodarysko Do you have it in *.bak file or in bytecode in /target/classes?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Tibor17 commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          @khospodarysko Something wrong happened. The IT class is missing.

          Show
          githubbot ASF GitHub Bot added a comment - Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/121 @khospodarysko Something wrong happened. The IT class is missing.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khospodarysko commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          Squashed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user khospodarysko commented on the issue: https://github.com/apache/maven-surefire/pull/121 Squashed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Tibor17 commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          @khospodarysko Can you squash two commits to one single commit?

          Show
          githubbot ASF GitHub Bot added a comment - Github user Tibor17 commented on the issue: https://github.com/apache/maven-surefire/pull/121 @khospodarysko Can you squash two commits to one single commit?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khospodarysko commented on the issue:

          https://github.com/apache/maven-surefire/pull/121

          Btw did not receive a failures after:
          mvn -Prun-its clean install
          or
          mvn clean install

          Show
          githubbot ASF GitHub Bot added a comment - Github user khospodarysko commented on the issue: https://github.com/apache/maven-surefire/pull/121 Btw did not receive a failures after: mvn -Prun-its clean install or mvn clean install
          Hide
          khospodarysko Kostya Hospodarysko added a comment -
          Show
          khospodarysko Kostya Hospodarysko added a comment - Create a pull request https://github.com/apache/maven-surefire/pull/121 .
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user khospodarysko opened a pull request:

          https://github.com/apache/maven-surefire/pull/121

          Fix for issue SUREFIRE-1278

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/khospodarysko/maven-surefire master

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/maven-surefire/pull/121.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 #121


          commit bbe5dd237a2b1ba1a353bb51361d35dfa5996733
          Author: Kostya Hospodarysko <khospodarysko@lohika.com>
          Date: 2016-09-15T22:08:42Z

          Fix for SUREFIRE-1278 - TestNG tests are run with group name that ends with specified group.

          commit 16bfc7c93f697b7a50ef2b041ba7e000754049c7
          Author: Kostya Hospodarysko <khospodarysko@lohika.com>
          Date: 2016-09-16T07:36:34Z

          Configured pom.xml to run only group "group" in order to show the issue.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user khospodarysko opened a pull request: https://github.com/apache/maven-surefire/pull/121 Fix for issue SUREFIRE-1278 You can merge this pull request into a Git repository by running: $ git pull https://github.com/khospodarysko/maven-surefire master Alternatively you can review and apply these changes as the patch at: https://github.com/apache/maven-surefire/pull/121.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 #121 commit bbe5dd237a2b1ba1a353bb51361d35dfa5996733 Author: Kostya Hospodarysko <khospodarysko@lohika.com> Date: 2016-09-15T22:08:42Z Fix for SUREFIRE-1278 - TestNG tests are run with group name that ends with specified group. commit 16bfc7c93f697b7a50ef2b041ba7e000754049c7 Author: Kostya Hospodarysko <khospodarysko@lohika.com> Date: 2016-09-16T07:36:34Z Configured pom.xml to run only group "group" in order to show the issue.
          Hide
          khospodarysko Kostya Hospodarysko added a comment -

          Thanks for help. Indeed it was the problem.

          Show
          khospodarysko Kostya Hospodarysko added a comment - Thanks for help. Indeed it was the problem.
          Hide
          gboue Guillaume Boué added a comment -

          It seems the issue is with the class SingleGroupMatcher, which determines if a given group is enabled by checking it if ends with the configured group.

          There are 2 overloads of the method, and the same "endsWith" test is done to determine if the given category, represented as a String or a Class, is enabled. There are test cases checking this behaviour for the Class... overload here, but not for the String... overload.

          Changing this method to use equals fixes the issue and does not break any existing test.

          Show
          gboue Guillaume Boué added a comment - It seems the issue is with the class SingleGroupMatcher , which determines if a given group is enabled by checking it if ends with the configured group. There are 2 overloads of the method, and the same "endsWith" test is done to determine if the given category, represented as a String or a Class , is enabled. There are test cases checking this behaviour for the Class... overload here , but not for the String... overload. Changing this method to use equals fixes the issue and does not break any existing test.
          Hide
          khospodarysko Kostya Hospodarysko added a comment -

          OK, I'll try to fix/debug in a couple of days and will make a pull request.

          Show
          khospodarysko Kostya Hospodarysko added a comment - OK, I'll try to fix/debug in a couple of days and will make a pull request.
          Hide
          tibor17 Tibor Digana added a comment -

          and you think the regex-based GroupMatcher is wrong?
          I guess you are able to fix it in pull request in Github.
          I am really preparing release. I am not sure if this is really Surefire
          problem because i did not have time to dig into debugging the code.

          On Thu, Sep 15, 2016 at 4:47 PM, Kostya Hospodarysko (JIRA) <jira@apache.org

          Show
          tibor17 Tibor Digana added a comment - and you think the regex-based GroupMatcher is wrong? I guess you are able to fix it in pull request in Github. I am really preparing release. I am not sure if this is really Surefire problem because i did not have time to dig into debugging the code. On Thu, Sep 15, 2016 at 4:47 PM, Kostya Hospodarysko (JIRA) <jira@apache.org
          Hide
          khospodarysko Kostya Hospodarysko added a comment -

          Tibor Digana testng-groups project does not cover the case I described. There is a method with group "abc-def". Assume that there is also a method with group "def". So after "mvn clean test -Dgroups=def" both methods are executed but I expect only one method with group "def" to be executed, not "abc-def".

          Show
          khospodarysko Kostya Hospodarysko added a comment - Tibor Digana testng-groups project does not cover the case I described. There is a method with group "abc-def". Assume that there is also a method with group "def". So after "mvn clean test -Dgroups=def" both methods are executed but I expect only one method with group "def" to be executed, not "abc-def".
          Show
          tibor17 Tibor Digana added a comment - For this we have integration tests https://github.com/apache/maven-surefire/tree/master/surefire-integration-tests/src/test/resources/testng-groups https://github.com/apache/maven-surefire/blob/master/surefire-integration-tests/src/test/java/org/apache/maven/surefire/its/TestNgGroupsIT.java Try to use Version 6.8.7.
          Hide
          khospodarysko Kostya Hospodarysko added a comment -

          Demo project.

          Show
          khospodarysko Kostya Hospodarysko added a comment - Demo project.

            People

            • Assignee:
              tibor17 Tibor Digana
              Reporter:
              khospodarysko Kostya Hospodarysko
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development