[SUREFIRE-1179] integration test

[Tibor17]

for #105

[Tibor17]

@juherr
This is the IT, but the problem is in TestNG because the concurrency remains 10 even if dataproviderthreadcount=30.
Meanwhile I reported issue in TestNG I found when I tested the clone of TestNG project for the IT and installed as snapshot version in my local repo. testng-team/testng#813
Can you make investigation and find out why we still have only 10 concurrent threads. Feel free to clone my branch and change the value of dataproviderthreadcount in POM.

[juherr]

Awesome! I should have time this week to close it.

[juherr]

I had time to analyze and I found that dataProviderThreadCount is taken into account only when m_commandLineTestClasses != null || m_commandLineMethods != null.

• m_commandLineTestClasses could only be != null when setTestClasses is called,
• m_commandLineMethods could only be != null when -methods is used, but -methods is not available via the deprecated TestNG#configure(Map cmdLineArgs).

I can't say if this behavior is expected or if something should be done in TestNG. What do you think @cbeust?

Another solution would be to update TestNGMapConfigurator#configure( XmlSuite suite, Map<String, String> options ) (or another Configurator) from surefire in order to use suite.setDataProviderThreadCount(). But the method is only available since 5.10 where the current TestNG used by surefire is 5.7.
But I don't know if I' allowed to update the dependency.

[Tibor17]

Since 5.10?
Hm, I was testing with 6.9.x and the number of threads was 10 instead of 30 configured in dataproviderthreadCount.
Pending question - we use small letters in param name i.e. dataproviderthreadCount. Does TestNG use CAMEL dataProviderThreadCount?

[juherr]

Since 5.10, suite.setDataProviderThreadCount() is available but surefire doesn't use it.

I'd like to add

        String dataProviderThreadCount = options.get( "dataproviderthreadcount" );
        if ( dataProviderThreadCount != null ) {
            suite.setDataProviderThreadCount( Integer.parseInt(dataProviderThreadCount) );
        }

in TestNGMapConfigurator#configure( XmlSuite suite, Map<String, String> options ) but I don't know if TestNGMapConfigurator is the best location to add it and if I'm allowed to update the TestNG dependency of surefire (from 5.7 to 5.10).

I've just tested by changing the value of dataproviderthreadcount in the test with 5 and 20, and it is working with the update I propose.

[juherr]

Sometime code is easier than explainations :) I updated #105. Feel free to comment.

[Tibor17]

All you need to do is to call this method with Java Reflection.
There is surefire-testng-util module.
You should create reflection utility in there.

Please be inspired by JUnit4ProviderUtil, ReflectionUtils and JUnit4Reflector.

Then create TestNgReflector class using ReflectionUtils with fail-fast
method setDataProviderThreadCount throwing SurefireReflectionException.
In your case the put message in the exception should be properly saying
that this method is supported since TestNG 5.10 or higher.

Do not forget the license header and Javadoc on the top of your class,
otherwise the rat-plugin will fail.

On Mon, Sep 28, 2015 at 3:26 PM, Julien Herr notifications@github.com
wrote:

Sometime code is easier than explainations :) I updated #105
#105. Feel free to comment.

—
Reply to this email directly or view it on GitHub
#106 (comment)
.

Cheers
Tibor

[Tibor17]

Is there the same principle with thread count for suites?
I did not test suite thread count because I do not have IT.
Can you try?

On Mon, Sep 28, 2015 at 4:25 PM, Tibor Digana tibor.digana@googlemail.com
wrote:

All you need to do is to call this method with Java Reflection.
There is surefire-testng-util module.
You should create reflection utility in there.

Please be inspired by JUnit4ProviderUtil, ReflectionUtils and JUnit4Reflector.

Then create TestNgReflector class using ReflectionUtils with
fail-fast method setDataProviderThreadCount throwing SurefireReflectionException.
In your case the put message in the exception should be properly saying
that this method is supported since TestNG 5.10 or higher.

Do not forget the license header and Javadoc on the top of your class,
otherwise the rat-plugin will fail.

On Mon, Sep 28, 2015 at 3:26 PM, Julien Herr notifications@github.com
wrote:

Sometime code is easier than explainations :) I updated #105
#105. Feel free to
comment.

—
Reply to this email directly or view it on GitHub
#106 (comment)
.

Cheers
Tibor

Cheers
Tibor

[juherr]

For my information, why not updating the testng dependency as I did it in #105?

I'll try to add some tests for suite thread count.

[Tibor17]

Because we were always dependent on 5.7 and the users may still rely on it.
It is question of one minute to crate Utility class in the module surefire-testng-util.
All you need to do is to have import section
org.apache.maven.surefire.util.ReflectionUtils.invokeSetter and call it.
Rethrow the exception with meaningful error message.

[juherr]

It's not a problem of time but I just want to understand what I'm doing.

I'm talking about an compile dependency (scoped provided) where, as I understand, users will use the version they want.
TestNG510Configurator should only be used with TestNG after 5.10 and cannot fail.

Am I missing something?

[Tibor17]

Alright create TestNG510Configurator and register it in AbstractSurefireMojo and increase the version in surefire-testng POM.

[Tibor17]

The style with reflection is usually in junit provider, but you'r right, forget reflection in testng.

[Tibor17]

There is already version range [5.3,6.4] you should split which means to split in
[5.3,5.10) and
[5.10,6.4] update only TestNGMapConfigurator.
Do you still need a new configurator if the method is available in 6.x?

[Tibor17]

@juherr I guess this was already merged in Version 2.19. You can close the pull-request. Thx

[juherr]

@Tibor17 No, I can't! YOU created this pull-request ;)

[Tibor17]

@juherr Sorry :), really it was me, sorry again 👍