Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cartesian product for data providers #865

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mrpotes
Copy link

@mrpotes mrpotes commented Nov 11, 2015

Taking some inspiration from this post on the forums, this change adds support for an array of data provider strings in the Test annotation.

@cbeust
Copy link
Collaborator

cbeust commented Nov 11, 2015

I think the overall idea can be useful but I'm not a fan of wiring it into the data provider engine since this can be done with straight Java by just combining the data providers manually and making that your data provider.

In other words, I think this could just be a helper method.

Thoughts?

@mrpotes
Copy link
Author

mrpotes commented Nov 11, 2015

It certainly could be a helper method (that's what I'm currently doing in
my tests), but I think it's neater to be built in as it saves on a lot of
unnecessary boilerplate, that gets in the way of readable tests. The neat
thing about it is that because of the way arrays in annotations work, it's
backwards-compatible for the user (although admittedly not for anyone doing
something more complicated with the TestNG API).

On 11 November 2015 at 18:10, Cedric Beust notifications@github.com wrote:

I think the overall idea can be useful but I'm not a fan of wiring it into
the data provider engine since this can be done with straight Java by just
combining the data providers manually and making that your data provider.

In other words, I think this could just be a helper method.

Thoughts?


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

@cbeust
Copy link
Collaborator

cbeust commented Nov 11, 2015

Another problem I have is that the syntax you chose to add (which is clever by the way, allowing multiple data provider names) can be interpreted in many ways. You chose to make it mean "cartesian product" but what if people just want to concatenate these results, for example?

It doesn't seem wise to hardcode how these multiple data providers are handled this way.

@mrpotes
Copy link
Author

mrpotes commented Nov 11, 2015

Yes, that occurred to me too - I could create an enum
dataProviderCombinator field? Combination types that occurred to me were:
PRODUCT, SIDE_BY_SIDE, CONCATENATE.

On 11 November 2015 at 20:23, Cedric Beust notifications@github.com wrote:

Another problem I have is that the syntax you chose to add (which is
clever by the way, allowing multiple data provider names) can be
interpreted in many ways. You chose to make it mean "cartesian product" but
what if people just want to concatenate these results, for example?

It doesn't seem wise to hardcode how these multiple data providers are
handled this way.


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

@cbeust
Copy link
Collaborator

cbeust commented Nov 11, 2015

An enum still restricts what can be done, and you need to implement all these strategies in TestNG, whether they ever get used or not.

All this points toward letting developers handle this themselves :)

@juherr
Copy link
Member

juherr commented Nov 12, 2015

And why not having a String[] dataProviders() into @DataProvider itself where the method will accept result of other data providers.
We can imagine TestNG will provide out-of-bo helper methods for the expected behavior.

Something like:

@DataProvider(dataProviders={"dp1", "dp2"})
public Object[][] dp(Object[][]... results) {
    return DataProviders.concat(results);
}

@ricardf-cmp
Copy link

@cbeust I've done this in a whole different helper class I've called TestcaseGenerator

@DataProvider(name="whUpgradeNewUserDataProvider")
    public static Object[][] whUpgradeNewUserDataProvider() {
        TestcaseGenerator tg = new TestcaseGenerator();

        tg.add(new IRunnableMethod() {
            @Override
            public Object run() {
                return WhUserBuilder.aRandomUser().build();
            }
        });
        tg.add("netbilling"); // gateway
        tg.add(System.currentTimeMillis()/1000 + 1000*60*60*24*365); // expiry
        tg.add("month"); // plan
        tg.add("descriptor_test", ""); // descriptor
        tg.add("support_site_test", ""); // support_site
        tg.add("555-111-222", ""); // support_phone

        return tg.generate();
    }

Also I'm planning to add Java 8 support so that "IRunnableMethod" would be way less verbose (using Callable functional interface + lambda expressions). Would you prefer that approach? You think it's useful to add that to TestNG? Current methods on TestcaseGenerator are: add, addOptional, clearParams, generate. Each one self explanatory.

@cbeust
Copy link
Collaborator

cbeust commented Dec 9, 2015

@ricardf-cmp TestNG needs to remain Java 1.7 compatible so no, we can't use lambdas or functional interfaces yet.

@ricardf-cmp
Copy link

@cbeust Maybe I did not use the right words. What I meant was I'm using this code in Java 1.7. Let me put a clearer example:

@DataProvider(name="sampleProvider")
    public static Object[][] sampleProvider() {
        TestcaseGenerator tg = new TestcaseGenerator();

        tg.add(new IRunnableMethod() {
            @Override
            public Object run() {
                return UserBuilder.aRandomUser().build();
            }
        });
        tg.add("normal String", "$%&", ""); // string example
        tg.addOptional(-1,0,1,99999); // optional numeric example

        return tg.generate();
    }
  • That IRunnableMethod will execute on every single user case. Useful when random parameters or time based parameters are needed. In my example I used a test data builder.
  • This will generate a total of 12 test cases:
    • First parameter cardinality = 1
    • Second parameter cardinality = 3
    • Third parameter cardinality = 4
    • 1 * 3 * 4 = 12

This is my last attempt :) If you don't like the idea then it's fine, for me it's been really helpful to stop thinking about test cases and just generate them all. If you think it's a good idea I'll create a PR for that.

@cbeust
Copy link
Collaborator

cbeust commented Dec 10, 2015

I think there are too many use cases to cover to justify adding this to TestNG, so I'd rather keep this out of the core.

@nitinverma
Copy link
Contributor

It'll be of more utility and lesser test cases for testng. If foo(dp...) is exposed.

@mouyang
Copy link

mouyang commented Feb 28, 2016

@mrpotes it would probably be cleaner to break up the cases you mentioned into separate annotations and keeping the current DataProvider annotation as the "simple" data provider. With an enum, all strategies would need to be implemented at once as @cbeust suggests but separate annotations could be implemented at your leisure. Thoughts? I'm thinking about moving forward with this.

@lbergelson
Copy link

This would be useful. At the very least baking in some methods to merge DataProviders would be convenient even if you didn't want to provide a full grammar for merging them in any possible way.

Needing to include methods for the Cartesian product of Object[][] in every project is kind of gross and this would be a natural place for them to live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants