You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by krmahadevan <gi...@git.apache.org> on 2017/10/09 04:06:12 UTC

[GitHub] maven-surefire pull request #166: Support filtering of tests from Base Class...

GitHub user krmahadevan opened a pull request:

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

    Support filtering of tests from Base Class (TestNG)

    User has two test classes
    1. An abstract base class called “BaseTest” which
    contains an @Test method called “testMethodFromBase”
    2. A child class called “ChildTest” which extends
    “BaseTest”.
    
    User attempts at running the TestNG test via
    -Dtest=ChildTest#testMethodFromBase”.
    
    Surefire plugin errors out.
    
    Fixed this anomaly for TestNG provider.

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

    $ git pull https://github.com/krmahadevan/maven-surefire krmahadevan-supportBaseClassTestFilteringUsingTestNG

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

    https://github.com/apache/maven-surefire/pull/166.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 #166
    
----
commit 9ed1e75700bd564968eaae444269f9bd280649a0
Author: Krishnan Mahadevan <kr...@gmail.com>
Date:   2017-10-09T04:05:11Z

    Support filtering of tests from Base Class (TestNG)
    
    User has two test classes
    1. An abstract base class called “BaseTest” which
    contains an @Test method called “testMethodFromBase”
    2. A child class called “ChildTest” which extends
    “BaseTest”.
    
    User attempts at running the TestNG test via
    -Dtest=ChildTest#testMethodFromBase”.
    
    Surefire plugin errors out.
    
    Fixed this anomaly for TestNG provider.

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...

Posted by Tibor17 <gi...@git.apache.org>.
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 #166: Support filtering of tests from Base Class (TestN...

Posted by krmahadevan <gi...@git.apache.org>.
Github user krmahadevan commented on the issue:

    https://github.com/apache/maven-surefire/pull/166
  
    @Tibor17 - Looks like a lot of things changed. Now after rebasing off of master, I dont even get IntelliJ to recognize the test classes that I added. So not sure what is going on there. 
    
    With respect to the formatting, I have tried importing into the IDE and applying it. But not sure if its actually changing the format.
    
    I had a lot of time at my hand when I raised this PR back in October, but now I am currently tied up with a lot of stuff at work. So I am not sure when I am gonna get to this. 


---

[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...

Posted by krmahadevan <gi...@git.apache.org>.
Github user krmahadevan commented on the issue:

    https://github.com/apache/maven-surefire/pull/166
  
    Changeset has been merged as part of https://github.com/apache/maven-surefire/commit/39acb98a94242149b5a8374a9619eb06f6b3cb6d
    
    Closing this PR. 
    
    @Tibor17 thanks for helping me with this PR.


---

[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...

Posted by Tibor17 <gi...@git.apache.org>.
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...

Posted by Tibor17 <gi...@git.apache.org>.
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...

Posted by krmahadevan <gi...@git.apache.org>.
Github user krmahadevan commented on the issue:

    https://github.com/apache/maven-surefire/pull/166
  
    ping @Tibor17 - This PR basically addresses only the *TestNG* portion of the bug https://github.com/cbeust/testng/issues/1563


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] maven-surefire pull request #166: Support filtering of tests from Base Class...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/166#discussion_r154519321
  
    --- Diff: surefire-providers/surefire-testng-utils/src/main/java/org/apache/maven/surefire/testng/utils/MethodSelector.java ---
    @@ -62,6 +62,17 @@ private static boolean shouldRun( ITestNGMethod test )
         {
             TestListResolver resolver = testListResolver;
             boolean hasTestResolver = resolver != null && !resolver.isEmpty();
    -        return hasTestResolver && resolver.shouldRun( test.getRealClass(), test.getMethodName() );
    +        boolean resolved = false;
    +        Class<?> clazz = test.getRealClass();
    +        if (resolver != null) {
    --- End diff --
    
    Please rework the code style according to Maven convention in our project and use `git commit --ammend`. There should be only single commit per PR and the message should refer to JIRA ID, something like:
    [SUREFIRE-12345] Support filtering of tests from Base Class (TestNG)
    and you can register at our Jira here:
    https://issues.apache.org/jira/projects/SUREFIRE


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...

Posted by Tibor17 <gi...@git.apache.org>.
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 #166: Support filtering of tests from Base Class (TestN...

Posted by krmahadevan <gi...@git.apache.org>.
Github user krmahadevan commented on the issue:

    https://github.com/apache/maven-surefire/pull/166
  
    @Tibor17 - I am not sure why it failed. I can't seem to tie down my changes to this failure.


---

[GitHub] maven-surefire issue #166: Support filtering of tests from Base Class (TestN...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on the issue:

    https://github.com/apache/maven-surefire/pull/166
  
    @krmahadevan 
    Sorry for late reply. I was ill last time and busy in the work.
    The code conventions are listed in https://maven.apache.org/developers/conventions/code.html
    In the middle of the page is XML for Eclipse and IDEA. You can use it to configure your tool.
    Then run the local build `mvn install -P run-its`. The build has 1400 tests, so it usually takes 50 minutes to complete on SSD disk.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] maven-surefire pull request #166: Support filtering of tests from Base Class...

Posted by Tibor17 <gi...@git.apache.org>.
Github user Tibor17 commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/166#discussion_r154519159
  
    --- Diff: surefire-providers/surefire-testng-utils/src/test/testng/utils/MethodSelectorTest.java ---
    @@ -0,0 +1,299 @@
    +package testng.utils;
    +
    +import junit.framework.TestCase;
    +import org.apache.maven.surefire.testng.utils.MethodSelector;
    +import org.apache.maven.surefire.testset.TestListResolver;
    +import org.testng.Assert;
    +import org.testng.IClass;
    +import org.testng.IRetryAnalyzer;
    +import org.testng.ITestClass;
    +import org.testng.ITestNGMethod;
    +import org.testng.internal.DefaultMethodSelectorContext;
    +
    +import java.lang.reflect.Method;
    +
    +public class MethodSelectorTest extends TestCase {
    +    public void testInclusionOfMethodFromBaseClass() {
    +        MethodSelector selector = new MethodSelector();
    +        DefaultMethodSelectorContext context = new DefaultMethodSelectorContext();
    +        ITestNGMethod testngMethod = new FakeTestNGMethod(ChildClassSample.class, "baseClassMethodToBeIncluded");
    +        TestListResolver resolver = new TestListResolver(BaseClassSample.class.getName() + "#baseClassMethodToBeIncluded");
    +        MethodSelector.setTestListResolver(resolver);
    +        boolean include = selector.includeMethod(context, testngMethod, true);
    +        Assert.assertTrue(include);
    +    }
    +
    +    private static class FakeTestNGMethod implements ITestNGMethod {
    +        private Class<?> clazz;
    +        private String methodName;
    +
    +        FakeTestNGMethod(Class<?> clazz, String methodName) {
    +            this.clazz = clazz;
    +            this.methodName = methodName;
    +        }
    +
    +        @Override
    +        public Class getRealClass() {
    +            return clazz;
    +        }
    +
    +        @Override
    +        public ITestClass getTestClass() {
    +            return null;
    +        }
    +
    +        @Override
    +        public void setTestClass(ITestClass iTestClass) {
    +
    +        }
    +
    +        @Override
    +        public Method getMethod() {
    +            return null;
    +        }
    +
    +        @Override
    +        public String getMethodName() {
    +            return this.methodName;
    +        }
    +
    +        @Override
    +        public Object[] getInstances() {
    +            return new Object[0];
    +        }
    +
    +        @Override
    +        public long[] getInstanceHashCodes() {
    +            return new long[0];
    +        }
    +
    +        @Override
    +        public String[] getGroups() {
    +            return new String[0];
    +        }
    +
    +        @Override
    +        public String[] getGroupsDependedUpon() {
    +            return new String[0];
    +        }
    +
    +        @Override
    +        public String getMissingGroup() {
    +            return null;
    +        }
    +
    +        @Override
    +        public void setMissingGroup(String s) {
    +
    +        }
    +
    +        @Override
    +        public String[] getBeforeGroups() {
    +            return new String[0];
    +        }
    +
    +        @Override
    +        public String[] getAfterGroups() {
    +            return new String[0];
    +        }
    +
    +        @Override
    +        public String[] getMethodsDependedUpon() {
    +            return new String[0];
    +        }
    +
    +        @Override
    +        public void addMethodDependedUpon(String s) {
    +
    +        }
    +
    +        @Override
    +        public boolean isTest() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isBeforeMethodConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isAfterMethodConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isBeforeClassConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isAfterClassConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isBeforeSuiteConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isAfterSuiteConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isBeforeTestConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isAfterTestConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isBeforeGroupsConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isAfterGroupsConfiguration() {
    +            return false;
    +        }
    +
    +        @Override
    +        public long getTimeOut() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public int getInvocationCount() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public void setInvocationCount(int i) {
    +
    +        }
    +
    +        @Override
    +        public int getSuccessPercentage() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public String getId() {
    +            return null;
    +        }
    +
    +        @Override
    +        public void setId(String s) {
    +
    +        }
    +
    +        @Override
    +        public long getDate() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public void setDate(long l) {
    +
    +        }
    +
    +        @Override
    +        public boolean canRunFromClass(IClass iClass) {
    +            return false;
    +        }
    +
    +        @Override
    +        public boolean isAlwaysRun() {
    +            return false;
    +        }
    +
    +        @Override
    +        public int getThreadPoolSize() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public void setThreadPoolSize(int i) {
    +
    +        }
    +
    +        @Override
    +        public String getDescription() {
    +            return null;
    +        }
    +
    +        @Override
    +        public void incrementCurrentInvocationCount() {
    +
    +        }
    +
    +        @Override
    +        public int getCurrentInvocationCount() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public void setParameterInvocationCount(int i) {
    +
    +        }
    +
    +        @Override
    +        public int getParameterInvocationCount() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public ITestNGMethod clone() {
    +            try {
    +                return (ITestNGMethod) super.clone();
    +            } catch (CloneNotSupportedException e) {
    +                throw new RuntimeException(e);
    +            }
    +        }
    +
    +        @Override
    +        public IRetryAnalyzer getRetryAnalyzer() {
    +            return null;
    +        }
    +
    +        @Override
    +        public void setRetryAnalyzer(IRetryAnalyzer iRetryAnalyzer) {
    +
    +        }
    +
    +        @Override
    +        public boolean skipFailedInvocations() {
    +            return false;
    +        }
    +
    +        @Override
    +        public void setSkipFailedInvocations(boolean b) {
    +
    +        }
    +
    +        @Override
    +        public long getInvocationTimeOut() {
    +            return 0;
    +        }
    +
    +        @Override
    +        public boolean ignoreMissingDependencies() {
    +            return false;
    +        }
    +
    +        @Override
    +        public void setIgnoreMissingDependencies(boolean b) {
    +
    +        }
    +
    +        @Override
    +        public int compareTo(Object o) {
    +            return 0;
    +        }
    +    }
    +}
    --- End diff --
    
    Pls add empty new line after this class. Thx.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


[GitHub] maven-surefire pull request #166: Support filtering of tests from Base Class...

Posted by krmahadevan <gi...@git.apache.org>.
Github user krmahadevan closed the pull request at:

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


---