testng 5.14.1 + Maven Surefire = not running junit tests

[you...@gmail.com]

I'm using maven. Everything in my project worked with testng 5.13.1.
When I upgraded to 5.14.1 I noticed that my junit tests aren't getting
run my testng surefire anymore. Anyone have any ideas?

I don't know if this is related but if I don't have any native testng
tests (only junit) in my maven project I get the following build
output by surefire:

-------------------------------------------------------
T E S T S
-------------------------------------------------------
Running TestSuite
[ERROR]: No test suite found. Nothing to run
Usage: <main class> [options]
The XML suite files to run
Options: -configfailurepolicy Configuration failure
policy (skip or continue)
-d Output directory
-dataproviderthreadcount Number of threads to use when
running data providers
-excludegroups Comma-separated list of group
names to exclude
-groups Comma-separated list of group
names to be run
-junit JUnit mode (default: false)
-listener List of .class files or list of
class names implementing ITestListener or ISuiteListener
-log, -verbose Level of verbosity
-methods Comma separated of test methods
(default: [])
-methodselectors List of .class files or list of
class names implementing IMethodSelector
-objectfactory List of .class files or list of
class names implementing ITestRunnerFactory
-parallel Parallel mode (methods, tests or
classes)
-port The port
-reporter Extended configuration for
custom report listener
-suitename Default name of test suite, if
not specified in suite definition file or source code
-suitethreadpoolsize Size of the thread pool to use
to run suites (default: 1)
-testclass The list of test classes
-testjar A jar file containing the tests
-testname Default name of test, if not
specified in suitedefinition file or source code
-testnames The list of test names to run
-testrunfactory, -testRunFactory The factory used to create tests
-threadcount Number of threads to use when
running tests in parallel
-usedefaultlisteners Whether to use the default
listeners (default: true)

Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.355
sec

[Cédric Beust ♔]

Can you share your pom.xml? I suspect you might have some invalid XML in it.

-- 

Cédric

--
You received this message because you are subscribed to the Google Groups "testng-users" group.
To post to this group, send email to testng...@googlegroups.com.
To unsubscribe from this group, send email to testng-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/testng-users?hl=en.

--
Cédric

[Stevo Slavić]

I've noticed as well that TestNG 5.14.x breaks compatibility with maven surefire and failsafe plugins (see SUREFIRE-648 issue with example attached).

Regards.
Stevo.

2010/10/4 Cédric Beust ♔ <ced...@beust.com>

[Cédric Beust ♔]

Stevo,

Can you explain why you think it's a TestNG and not a Surefire issue?

Thanks.

-- 

Cédric

[Stevo Slavić]

I do think it's surefire issue, that's why I reported that issue on surefire issue tracker.

Regards,

[Cédric Beust ♔]

[+Brett]

Ok, I was confused because you said "TestNG 5.14.x breaks compatibility with maven surefire", which worried me.

Brett, are you aware of this issue? Anything I can do to help?

-- 

Cédric

[you...@gmail.com]

Steps to reproduce:
1. Created a new project using the "maven-archetype-quickstart"
2. Add testng 5.14.1 to the dependency list.
3. Run mvn clean install and you will see that the default "AppTest"
is not run and that the output I posted earlier and no tests run.

Thanks,
Mike

Here is what the pom looks like:

<project xmlns="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">
<modelVersion>4.0.0</modelVersion>

<groupId>test</groupId>
<artifactId>test</artifactId>
<version>1.0</version>
<packaging>jar</packaging>

<name>test</name>
<url>http://maven.apache.org</url>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
</properties>

<dependencies>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>3.8.1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testng</groupId>
<artifactId>testng</artifactId>
<version>5.14.1</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>

> > testng-users...@googlegroups.com<testng-users%2Bunsubscribe@google groups.com>
> > .

[you...@gmail.com]

It appears Brett has confirmed this problem in: http://jira.codehaus.org/browse/SUREFIRE-648

Mike

On Oct 4, 5:37 pm, "you...@gmail.com" <you...@gmail.com> wrote:
> Steps to reproduce:
> 1. Created a new project using the "maven-archetype-quickstart"
> 2. Add testng 5.14.1 to the dependency list.
> 3. Run mvn clean install and you will see that the default "AppTest"
> is not run and that the output I posted earlier and no tests run.
>
> Thanks,
> Mike
>
> Here is what the pom looks like:
>
> <project xmlns="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.0http://maven.apache.org/xsd/maven-4.0.0.xsd">

[Brett Porter]

I hadn't had a chance to look until now. TestNG configuration is not working with 5.14.1. I can confirm the two issues listed here so far, as well as the integer parameters causing a ClassCastException (which is echoed by the "TODO" in TestNG.java). From a quick glance it also looks like excludedgroups changed to excludegroups which would affect this and any command line users on the old parameters.

As far as I can tell, this isn't a Surefire bug. It's likely TestNG changed the way it interprets the public API for configure( Map ). The question is whether compatibility should be restored in TestNG, or if Surefire needs to adjust to the new expectations. Sorry I don't have much more time to dig down to the root cause right now.

This was a bit hard to track to the related change, since TestNG's JIRA doesn't have version numbers for issues, and there isn't an SVN tag for 5.13.1 or 5.14.1 (the last is 5.12). Am I missing something?

- Brett

2010/10/5 Cédric Beust ♔ <ced...@beust.com>

--
Brett Porter
http://brettporter.wordpress.com/

[Stevo Slavić]

It's a bug in org/testng/TestNG.java, "public void configure(Map cmdLineArgs)" method constructs "CommandLineArgs result = new CommandLineArgs();" and builds it based on passed in "Map cmdLineArgs" and doesn't do anything with "result". It seems to me a call to "protected void configure(CommandLineArgs cla)" is missing at the end of the "public void configure(Map cmdLineArgs)"

Regards,
Stevo.

[Cédric Beust ♔]

Hi Stevo,

Indeed. I am stumped, a quick look at the log doesn't tell me when that happened, and I also don't understand why 5.14.1 is still working fine despite this. I need to run out but I'll get to the bottom of that.

-- 

Cédric

[Cédric Beust ♔]

I think I figured it out.

Because pom's can't have circular dependencies, I need to use two different pom's for TestNG: one to build everything but not run the tests (pom.xml) and one to just run the tests (pom-test.xml).

In order for this to work, pom-test.xml needs to be exactly one version ahead of pom.xml.

When I pushed 5.14.1, I updated pom.xml but I think I forgot to update pom-test.xml, which was therefore still running against 5.14.

I should probably think of way to verify this in the build.

I just pushed 5.14.2, which should solve the problem, try it and let me know.

-- 

Cédric

2010/10/4 Cédric Beust ♔ <ced...@beust.com>

--
Cédric

[Brett Porter]

On 05/10/2010, at 1:46 PM, Cédric Beust ♔ wrote:

> I think I figured it out.
>
> Because pom's can't have circular dependencies, I need to use two different pom's for TestNG: one to build everything but not run the tests (pom.xml) and one to just run the tests (pom-test.xml).
>
> In order for this to work, pom-test.xml needs to be exactly one version ahead of pom.xml.
>
> When I pushed 5.14.1, I updated pom.xml but I think I forgot to update pom-test.xml, which was therefore still running against 5.14.
>
> I should probably think of way to verify this in the build.
>
> I just pushed 5.14.2, which should solve the problem, try it and let me know.

It resolves that particular problem.

I've rejigged the Surefire integration tests to see what other regressions could be detected. There are 3, all starting in 5.13:

1) In 5.12.1, the following gives an error due to the cycle:

@Test(groups = { "test" }, dependsOnGroups = { "test" })

In 5.13.1+, the test is skipped. Is that intentional? One of the test cases was using it to test what happens when TestNG fails early.

2) In 5.12.1, the following is valid configuration:

<property><name>listener</name><value>listenReport.ResultListener,listenReport.SuiteListener</value></property>
<property><name>reporter</name><value>listenReport.Reporter</value></property>

In 5.13.1+, the API has changed such that both expect to be an ArrayList instead of a String. Can TestNG do an instanceof check and convert if the "old" string mechanism is used?

3) Likewise,

<threadCount>3</threadCount>

is passed in as a String (and there is one more example though we don't have a test for it). This causes a class cast exception, expecting an integer. Likewise, can it be converted?

- Brett

[Brett Porter]

BTW, to confirm (or check in future) the same results you can now do this:

svn co http://svn.apache.org/repos/asf/maven/surefire/trunk surefire
cd surefire
mvn clean install -Dtestng.version=5.14.2

You can use that for versions back to TestNG 4.7 I think to compare results. In the future I'll probably set the individual tests to only run on the ones they are known to work for.

Cheers,
Brett

--
Brett Porter
br...@apache.org
http://brettporter.wordpress.com/

[Cédric Beust ♔]

Hi Brett, and thanks for taking the time to look into these backward compatibility issues.

On Tue, Oct 5, 2010 at 1:45 AM, Brett Porter <br...@apache.org> wrote:

>
> I just pushed 5.14.2, which should solve the problem, try it and let me know.

It resolves that particular problem.

Great.

 

I've rejigged the Surefire integration tests to see what other regressions could be detected. There are 3, all starting in 5.13:

1) In 5.12.1, the following gives an error due to the cycle:

@Test(groups = { "test" }, dependsOnGroups = { "test" })

In 5.13.1+, the test is skipped. Is that intentional? One of the test cases was using it to test what happens when TestNG fails early.

I don't think there was any change in that area, but something that could have caused the change in behavior is whether there used to be more than one method in the group "test" and now there is only one. Do you think you might have done that in your test?

I agree it's not an intuitive behavior, though, and more specifically, the bug is that when TestNG only finds one test method, it just skips the sorting phase (why sort if there's only one method?). However, it's the sorting phase that detects cycles :-)

I just fixed this bug and pushed to master.

 

2) In 5.12.1, the following is valid configuration:

           <property><name>listener</name><value>listenReport.ResultListener,listenReport.SuiteListener</value></property>
           <property><name>reporter</name><value>listenReport.Reporter</value></property>

In 5.13.1+, the API has changed such that both expect to be an ArrayList instead of a String. Can TestNG do an instanceof check and convert if the "old" string mechanism is used?

You are right about the first observation, and the code now does:

    Object listeners = cmdLineArgs.get(CommandLineArgs.LISTENER);
    if (listeners instanceof List) {
      result.listener = Utils.joinClasses((List<Class>) listeners, " ");
    } else {
      result.listener = (String) listeners;
    }

However, for reporters, TestNG still seems to expect a string:

    String reporterConfigs = (String) cmdLineArgs.get(CommandLineArgs.REPORTERS_LIST);
    if (reporterConfigs != null) {
      result.reportersList = reporterConfigs;
    }

Can you double check in TestNGMapConfigurator?

Speaking of this class: it doesn't seem to be consistent with the way TestNG supports and parses collections of the same property. For example, it looks like you can only specify one listener with Surefire, or did I misread the code?  (didn't look too deep)

 

3) Likewise,

<threadCount>3</threadCount>

is passed in as a String (and there is one more example though we don't have a test for it). This causes a class cast exception, expecting an integer. Likewise, can it be converted?

Sure (and there's indeed a TODO comment in the TestNG code about that, although I can't recall if I wrote it or if someone else did).

Since I seem to release more often than Surefire, I guess I should make the change to string for now. Down the line, we should modify Surefire to invoke the new configure() method (the one that accepts a CommandLineArgs class), which is much safer from a typing standpoint.

Here is the new code:

    String threadCount = (String) cmdLineArgs.get(CommandLineArgs.THREAD_COUNT);
    if (threadCount != null) {
      result.threadCount = Integer.parseInt(threadCount);
    }

I also notice that Surefire doesn't seem to support "-dataproviderthreadcount", but I added support for it in TestNG anyway.

Here is the change.

--
Cédric

[Brett Porter]

On 06/10/2010, at 5:24 AM, Cédric Beust ♔ wrote:

I've rejigged the Surefire integration tests to see what other regressions could be detected. There are 3, all starting in 5.13:

1) In 5.12.1, the following gives an error due to the cycle:

@Test(groups = { "test" }, dependsOnGroups = { "test" })

In 5.13.1+, the test is skipped. Is that intentional? One of the test cases was using it to test what happens when TestNG fails early.

I don't think there was any change in that area, but something that could have caused the change in behavior is whether there used to be more than one method in the group "test" and now there is only one. Do you think you might have done that in your test?

I agree it's not an intuitive behavior, though, and more specifically, the bug is that when TestNG only finds one test method, it just skips the sorting phase (why sort if there's only one method?). However, it's the sorting phase that detects cycles :-)

I just fixed this bug and pushed to master.

That works, thanks.

 

2) In 5.12.1, the following is valid configuration:

           <property><name>listener</name><value>listenReport.ResultListener,listenReport.SuiteListener</value></property>
           <property><name>reporter</name><value>listenReport.Reporter</value></property>

In 5.13.1+, the API has changed such that both expect to be an ArrayList instead of a String. Can TestNG do an instanceof check and convert if the "old" string mechanism is used?

You are right about the first observation, and the code now does:

    Object listeners = cmdLineArgs.get(CommandLineArgs.LISTENER);
    if (listeners instanceof List) {
      result.listener = Utils.joinClasses((List<Class>) listeners, " ");
    } else {
      result.listener = (String) listeners;
    }

The subsequent split call uses ';' or ',', not ' ' - so the above code causes the following:

Cannot load class from file: listenReport.ResultListener listenReport.SuiteListener

at org.testng.internal.ClassHelper.fileToClass(ClassHelper.java:464)

at org.testng.TestNG.configure(TestNG.java:1182)

at org.testng.TestNG.configure(TestNG.java:1343)

at org.apache.maven.surefire.testng.conf.TestNGMapConfigurator.configure(TestNGMapConfigurator.java:97)

If " " is changed to "," all the tests pass.

However, for reporters, TestNG still seems to expect a string:

    String reporterConfigs = (String) cmdLineArgs.get(CommandLineArgs.REPORTERS_LIST);
    if (reporterConfigs != null) {
      result.reportersList = reporterConfigs;
    }

Can you double check in TestNGMapConfigurator?

Actually, it seems this was fine - I might have messed up this test since I was trying to avoid the above problem and it's in the same POM.

Speaking of this class: it doesn't seem to be consistent with the way TestNG supports and parses collections of the same property. For example, it looks like you can only specify one listener with Surefire, or did I misread the code?  (didn't look too deep)

They're supported in the same way as listeners, the example only lists one.

 

3) Likewise,

<threadCount>3</threadCount>

is passed in as a String (and there is one more example though we don't have a test for it). This causes a class cast exception, expecting an integer. Likewise, can it be converted?

Sure (and there's indeed a TODO comment in the TestNG code about that, although I can't recall if I wrote it or if someone else did).

That's working too, thanks!

Since I seem to release more often than Surefire, I guess I should make the change to string for now. Down the line, we should modify Surefire to invoke the new configure() method (the one that accepts a CommandLineArgs class), which is much safer from a typing standpoint.

[...]

I also notice that Surefire doesn't seem to support "-dataproviderthreadcount", but I added support for it in TestNG anyway.

Actually, Surefire stopped adding TestNG specific parameters to its configuration after the initial set. When Alex implemented the map configurator, we added a <properties> element to Surefire that let's you pass in any argument you want (such as the above).

It's intentional that we shouldn't have to release a new Surefire every time that TestNG comes out, so that users can choose which version suits them for TestNG.

I like the idea of CommandLineArgs variant, especially if we can expose that to the plugin configuration so users can get better feedback on the right args to use. But we have to be careful about binding Surefire to a particular TestNG release.

Anyway, there's just that one fix left above - thanks for acting on it so quickly!

Cheers,

Brett

[Cédric Beust ♔]

Hi Brett,

On Tue, Oct 5, 2010 at 5:54 PM, Brett Porter <br...@apache.org> wrote:

The subsequent split call uses ';' or ',', not ' ' - so the above code causes the following:

Cannot load class from file: listenReport.ResultListener listenReport.SuiteListener

at org.testng.internal.ClassHelper.fileToClass(ClassHelper.java:464)

at org.testng.TestNG.configure(TestNG.java:1182)

at org.testng.TestNG.configure(TestNG.java:1343)

at org.apache.maven.surefire.testng.conf.TestNGMapConfigurator.configure(TestNGMapConfigurator.java:97)

If " " is changed to "," all the tests pass.

Good catch, fixed.

I also notice that Surefire doesn't seem to support "-dataproviderthreadcount", but I added support for it in TestNG anyway.

Actually, Surefire stopped adding TestNG specific parameters to its configuration after the initial set. When Alex implemented the map configurator, we added a <properties> element to Surefire that let's you pass in any argument you want (such as the above).

It's intentional that we shouldn't have to release a new Surefire every time that TestNG comes out, so that users can choose which version suits them for TestNG.

I like the idea of CommandLineArgs variant, especially if we can expose that to the plugin configuration so users can get better feedback on the right args to use. But we have to be careful about binding Surefire to a particular TestNG release.

Indeed, your approach makes sense.

If I understand things correctly, if we keep using configure(Map) as the connection between Surefire and TestNG, then:

Pro:

Maven automatically handles new parameters via <properties>.

Con:

We lose some type safety and we need to make sure that TestNG and Surefire agree on the types of the parameters passed in the Map (which can lead to the class cast exceptions we are currently facing).

If we deprecate configure(Map) and have Surefire use configure(CommandLineArgs) instead, then:

Pro:

No more risk of class cast exceptions.

Con:

Surefire needs to be updated whenever new parameters are added.

I think this last point is the deal breaker. Even if it means more work for me, I think it's safer to keep going with the first option because TestNG releases more often than Surefire. I just need to remember to update the two configure() methods whenever I add a new parameter.

Is my analysis correct?

--
Cédric

[Brett Porter]

On 07/10/2010, at 6:14 AM, Cédric Beust ♔ wrote:

> I think this last point is the deal breaker. Even if it means more work for me, I think it's safer to keep going with the first option because TestNG releases more often than Surefire. I just need to remember to update the two configure() methods whenever I add a new parameter.
>
> Is my analysis correct?

I think so, but let me take a look and see if there's anything I could work out to get CommandlineArgs into newer versions.

- Brett