ceylon.test: support parametrized tests

[thradec]

see discussion in #158

[lucaswerkmeister]

I think we derailed a bit in that discussion :-) here’s a summary:

@thradec proposed the following syntax here:

{File*} testFiles() => testFileDirectory.files("*.ceylon");

test
void shouldFormatCode(dataProvider(`testFiles`) File f) {
    ...
}

This requires ceylon/ceylon-spec#847.

[russel]

Data-driven testing capability is critical. Spock enables this via the where: clause, and usually an @Unroll AST transform. TestNG makes use of "data providers". JUnit has the (awkward in my view) Parameterized annotation. Spock (via standard Groovy code) and TestNG (via a special method) allow cross-products over data sets which turns out to be extremely useful.

[quintesse]

Could you explain a bit more? I'm curious about what this is exactly and how it works, but the above could just as well been written in Chinese for me :)

[thradec]

FTR this issue is blocked by current annotations limitation, see ceylon/ceylon-spec#847

[russel]

@quintesse The (not entirely) trivial example is testing 4 different algorithms for calculating factorial. There are an infinite number of tests for full testing, so you have to make a selection from the domain. Say -100, -10, -1, 0, 1, 4, 10, 100, 1000. Rather than write 36 methods manually, I write 2 methods and some tabular data, and some meta-object protocol (i.e. compiler or run time infrastructure) rewrites my code to generate the 36 methods at run time. This is really trivial with Spock/Groovy, and Python/Pytest, doable with TestNG, and I have never tried with JUnit as I have given up on using it.

[quintesse]

Ok, but I still don't see the advantage, couldn't what you describe be done with a for loop over the data-set and 4 calls to the 4 implementations?

[quintesse]

Although I guess that what you want is that they really show up as different tests, a bit like we do for the language tests for Ceylon in JUnit where we generate our own list of tests on the fly.

[russel]

@quintesse looping with four asserts is what has to be done in languages that do not do things correctly! The issue here is one test case means one test method. This way all the tests run all the time, each fail is independent. With loops, you fail on first assert fail and the other tests are not run.

[luolong]

Well @quintesse, to reiterate what @russel said, with parametrized tests you write a test method as usual, except that it has a nonempty parameter list.

Test runner will take that method, create a bunch of test cases based on the data provider for that parameter (see the sample code by lucas above) and run them each one by one as a separate test cases.

The advantage of this over for loop is that if your test fails with one use case, you get test failure only for that case and all other cases will pass.

[quintesse]

Thanks @luolong , I now actually understand that example ;)

[FroMage]

An API would be trivial to support:

test
shared void normalTest(){}

test
shared void customTest(TestApi api){
 api.testParameterised(myTest, 2);
 api.testParameterised(myTest, 3);
}

void myTest(Integer param){
//...
}

Where TestApi is defined in ceylon.test and uses the given function reference to run the test and add it to the list of tests results, etc…

[quintesse]

@FroMage exactly, I probably lean more towards doing thing explicitly myself ;)

[russel]

@FroMage That sort of code will just turn people off. The TestNG, Spock and PyTest ways of doing things would be far more suitable. The issue here is that the data must be provided as some table structure not as code. I guess I should publish my Factorial implementations to avoid replicating things here.

[FroMage]

I'm not saying this is a better way, but a very possible workaround that can be implemented now until we support what @thradec needs to implement more powerful annotations.

[PhiLhoSoft]

FTR, I successfully converted a parametrized TestNG test to a parametrized JUnit test: https://github.com/PhiLhoSoft/Java/blob/master/Utilities/test/org/philhosoft/util/TestIntArrayList.java Check History to see the diffs.

I find the TestNG way, passing parameters to test, nicer, although experience shows that Eclipse doesn't like to jump to such test... But defining fields in the test class isn't so bad, perhaps.

Just to say it can be an alternative for Ceylon...

JUnit shows the runs of the test like:

org.philhosoft.util.TestIntArrayList [Runner: JUnit 4] (2.549 s)
  > v[0] (0.195 s)
  > v[1] (0.105 s)
  > v[2] (0.046 s)
  > x[3] (0.106 s)
      x testSort[3] (0.105 s)
      v testSort2[3] (0.000 s)
      v testAddRemove[3] (0.000 s)

Ie. indeed it highlights only the test that failed for a given parameter. (v = OK; x = failed)

[lucaswerkmeister]

I hacked together an implementation of this (outside of ceylon.test) that uses declarations instead of models. Supports toplevel nullary functions or values as data providers, works with any number of arguments (cartesian product). https://gist.github.com/lucaswerkmeister/0f618ae934359443f963

What would be the advantages of doing this with models instead of declarations? If ceylon/ceylon-spec#847 / ceylon/ceylon#3953 is going to be delayed for much longer, I think it might be worth adding a declaration-based version of this feature soon, and then potentially switch to a model-based version later, when it’s possible.

[thradec]

I hacked together an implementation of this (outside of ceylon.test)

now I'm waiting for proper PR :wink:

What would be the advantages of doing this with models instead of declarations?

just a little more type safety, we would be able to restrict, that data provider doesn't take any parameters and returns sequence/iterable (and little less typing)

I think it might be worth adding a declaration-based version of this feature soon

agree, I expected that annotations improvements will be targeted shortly after its first version, but it doesn't seem it would be done soon

[lucaswerkmeister]

now I'm waiting for proper PR :wink:

I knew that was coming! :D I might have some time for it this weekend, hopefully.

we would be able to restrict, that data provider doesn't take any parameters

Actually, I just realized that a data provider could take parameters… as long as they themselves have data providers. I suspect there are actually useful applications of this.

[lucaswerkmeister]

So I thought about this some more, and realized there’s at least one more way to fill in test function parameters, in addition to data providers: generate random objects. You could consider this a kind of fuzz testing, I think.

Given this, I see two possible ways to abstract over this (new interface in addition to TestRunner, TestExecutor and friends):

1. Something that takes a list of parameter declarations and returns a list of argument lists. ceylon.test then constructs one TestExecutor per argument list and runs the tests.
2. Something that takes a single parameter declaration and returns a list of arguments. ceylon.test then forms the cartesian product of all arguments for all parameters, constructs one TestExecutor per argument list, and runs the tests.

I’m not sure which one is better – I feel like either is more flexible than the other in some aspect. 2 lets you combine different “argument providers” for different arguments of the same function (e. g. one parameter with dataProvider, one with randomly generated arguments). But 1 lets you run tests for only some of the combinations. Thoughts?

[thradec]

Without deeper thinking, I'm not sure if to go with 1. or 2. or if there is some other way. But you are right, that we have to consider another sources of parameter values (data providers, random values, values injected from DI containers, mocks, ...).

Another aspect is, if the test plan (tree of tests) should be static, or if we want to allow dynamically adding tests.

[lucaswerkmeister]

I decided to go with 1 for now; I think 2 can be emulated with 1, which isn’t true vice versa. The abstraction happens here:

TestExecutor createExecutor(FunctionDeclaration funcDecl, ClassDeclaration? classDecl) {
    value argumentProvider = DefaultTestArgumentProvider(); // TODO add mechanism to choose argument provider
    value executorAnnotation = findAnnotation<TestExecutorAnnotation>(funcDecl, classDecl);
    value executorClass = executorAnnotation?.executor else `class DefaultTestExecutor`;
    value executors = [
        for (arguments in argumentProvider.argumentLists(funcDecl, classDecl))
            if (is TestExecutor executor = executorClass.instantiate([], funcDecl, classDecl, arguments))
                executor
    ];
    if (nonempty executors) {
        if (executors.size == 1) {
            return executors.first;
        } else {
            return GroupTestExecutor(TestDescription(funcDecl.qualifiedName, funcDecl, null, null, executors*.description), executors.sequence());
        }
    } else {
        return ErrorTestExecutor(TestDescription(funcDecl.qualifiedName, funcDecl), Exception("test argument provider ``argumentProvider`` did not provide any argument lists for test function ``funcDecl.qualifiedName``"));
    }
}

That is, createExecutor can return a GroupExecutor if the argumentProvider provides multiple argument lists. I still need to write a way to choose the TestArgumentProvider class (just like you can choose the TestExecutor class), and DefaultTestArgumentProvider currently just returns [[]] – it needs to inspect the parameters of course, to check the dataProviders. But the rest already works – with the extra abstraction in place, nullary tests can still be run. (For non-nullary test functions, the invoke call in the TestExecutor will fail. The error message is actually pretty good, telling you how many arguments were expected and given.) I’ll probably be able to finish the missing bits tomorrow.

[lucaswerkmeister]

So I have something that works now. No custom TestArgumentProviders yet, but dataProviders work. All my usual tests in ceylon.ast work, and I was able to rewrite the ceylon.formatter tests to be parameterized.

However, there’s a large number of test failures in the ceylon.test tests. And investigating them is a bit tricky at the moment, given IDE problems like ceylon/ceylon-ide-eclipse#1644. Might take me a while to fix these.

I’ve pushed what I have so far on my parameterizedTest branch if you want to take a look.

[thradec]

Thanks @lucaswerkmeister, I'm going look at it.

[thradec]

I looked at it briefly and I'm curious mainly about

• How do you plan to support alternative sources of arguments values?
• Did you consider adding tests dynamically? (the initial time for first test execution is relatively big, mainly due metamodel initialisation, and generating all variants will prolong it more)

(others minor comments are inline)

[lucaswerkmeister]

How do you plan to support alternative sources of arguments values?

Same as TestExecutor, I think. Annotation on the test function (or containing declaration?) to choose the TestArgumentProvider class; is instantiated with test FunctionDeclaration and ClassDeclaration?.

Did you consider adding tests dynamically?

Not sure what you mean, sorry…

[thradec]

Same as TestExecutor, I think. Annotation on the test function (or containing declaration?) to choose the TestArgumentProvider class; is instantiated with test FunctionDeclaration and ClassDeclaration?.

but in that case, you will not be able to combine different argument sources in one test function (maybe I don't get it right, example would help)

[lucaswerkmeister]

Each TestArgumentProvider gives an entire argument list. It can do that by combining multiple other test sources; for example, you could write a TestArgumentProvider that works on functions with parameter types T, Integer, and assigns the first parameter via dataProvider and the second one by choosing a random Integer.

[thradec]

Each TestArgumentProvider gives an entire argument list. It can do that by combining multiple other test sources; for example, you could write a TestArgumentProvider that works on functions with parameter types T, Integer, and assigns the first parameter via dataProvider and the second one by choosing a random Integer.

but the "randomized source" comes from module A and "DI source" comes from module B, and I don't want to write TestArgumentProvider which will integrate them, because somebody else would like to use some cool "source" from module C

[thradec]

maybe example will show, what I have in mind

test
shared void f(dataProvider(`intCornerCases`) Integer i, random String s, inject Connection c )

each annotation will satisfy some ceylon.test contract, so it will know how to resolve their values

[thradec]

Did you consider adding tests dynamically?

Not sure what you mean, sorry…

(sorry, my fault) now the TestDescriptor tree is created at the beginning and it is static, I wonder, if won't be better to create child nodes with concrete arguments values when the test itself is executed

[lucaswerkmeister]

(edit: in reference to this comment)

Yes, and this is supported if you provide a TestArgumentProvider that checks the annotations for each parameter, and

• gets arguments for dataProvider parameters from the DefaultTestArgumentProvider,
• gets arguments for random parameters from module A’s RandomTestArgumentProvider,
• gets arguments for inject parameters from module B’s DITestArgumentProvider,
• combines them and returns the resulting list of argument lists.

It can get arguments per parameter from the different argument providers by asking each of them for arguments to the parameter list with only a single parameter. That is, to get arguments for parameter p in parameters, use DefaultTestArgumentProvider().argumentLists([p]) .

That is complicated, and ceylon.test could do it for you, yes. But what this allows you to do that approach 2 can’t do is that this TestArgumentProvider can now also filter the argument lists. For example, it could only provide argument lists where i > s.length.

Idea: The dataProvider annotation (or annotation class) is annotated with some other annotation that specifies the TestArgumentProvider that processes this annotation on a single parameter. For instance, dataProvider’s annotation points to the class that is currently called DefaultTestArgumentProvider (though then I’ll probably rename it). Then, we provide another TestArgumentProvider that checks the annotations of each parameter, tries to obtain a TestArgumentProvider for that parameter based on the annotation’s annotation, and combines all the individual arguments into argument lists. This would alow you to write your example as above, but still give you the flexibility to operate on the entire argument list level if you want to.

(I hope I’m not explaining this too terribly; sounds complicated…)

[thradec]

I can't say I understand it ;-), but I hope, we have same goal.

[thradec]

Idea: The dataProvider annotation (or annotation class) is annotated with some other annotation ...

look how IgnoreAnnotation is implemented, maybe we can use similar approach

[thradec]

one more thing, it would be nice if this mechanism can be used also for before/after callbacks

[thradec]

another approach to "data providers" might be, to allow TestExecutor run test several times and report more then one result, instead of creating new TestExecutor for each arguments combination (just a thought for discussion ;-))

[thradec]

@lucaswerkmeister FYI I started prototype my last thought, to see how far it goes

[thradec]

first raw version is here d8d88ee

[lucaswerkmeister]

I like it, it’s pretty close to what I had in mind: annotations satisfy an interface to provide a single argument, and there’s an interface to provide full argument lists with a default implementation that just combines arguments from annotations.

Random comments:

1. Both do the same thing IMO, just on different scales, so I would’ve called them ArgumentProvider and ArgumentsProvider, not Resolver.
2. An argument list should be a sequential (Anything[]), not a stream. This would also serve as documentation: Currently, you might think that ArgumentsResolver.resolve() returns a stream of ArgumentProvide.values() results.
3. For DefaultArgumentsResolver.calculateArgumentVariants, I suggest you use my EndList. I think it should be pretty efficient, especially on memory (since it reuses as much as possible), but also on CPU (since it never copies).
4. DataProviderAnnotation.values could just return inside the case blocks.
5. I assume a mechanism to choose different argsResolvers will be added?
6. verifyFunctionParameters should just be removed, I think. There’s no way you can verify that the arguments resolver gives you properly typed arguments, especially not if you’re doing it all lazily.
7. Why not support multiple ArgumentProvider annotations per parameter? Just concatenate the results in resolveArgumentProvider.

[thradec]

1. Both do the same thing IMO, just on different scales, so I would’ve called them ArgumentProvider and ArgumentsProvider, not Resolver.

Not so much same from my point of view. ArgumentProvider is supposed to be widely implemented by different annotations. ArgumentResolver is much closer to "engine", it should take care about verification and orchestration several ArgumentProviders. And also I wanted to have opened door, and be able introduce ArgumentsProvider which will return values for all parameters.

1. An argument list should be a sequential (Anything[]), not a stream ...
2. For DefaultArgumentsResolver.calculateArgumentVariants, I suggest you use my EndList ...
3. DataProviderAnnotation.values could just return inside the case blocks ...

yes, yes and yes

1. I assume a mechanism to choose different argsResolvers will be added?

I plan to refactor several SPI interfaces as this one, and come with unified registration mechanism.

1. verifyFunctionParameters should just be removed, I think. There’s no way you can verify that the arguments resolver gives you properly typed arguments, especially not if you’re doing it all lazily.

there should be at least some verification, that every parameter has assigned ArgumentProvider and provide reasonable error message to user, if not

1. Why not support multiple ArgumentProvider annotations per parameter? Just concatenate the results in resolveArgumentProvider.

I would rather want to hear why yes ;-) I'm afraid of confusion, with usage several ArgumentProviders above one parameter. But it is doable.

btw. last but not least, thanks for review/comments :-)

[lucaswerkmeister]

And also I wanted to have opened door, and be able introduce ArgumentsProvider which will return values for all parameters.

Isn’t this exactly what ArgumentResolver does? It doesn’t have to rely on individual parameter annotations…

there should be at least some verification, that every parameter has assigned ArgumentProvider and provide reasonable error message to user, if not

I could provide a custom ArgumentResolver/ArgumentsProvider that doesn’t need any ArgumentProvider annotations.

[thradec]

I could provide a custom ArgumentResolver/ArgumentsProvider that doesn’t need any ArgumentProvider annotations.

Verification logic will be in ArgumentsResolver, which is also its added value over ArgumentsProvider.

[lucaswerkmeister]

What verification? You put the arguments in invoke and let the metamodel verify them, I don’t see what additional verification you’d want to do yourself.

[thradec]

eg. check that all parameters have assigned arg.provider, check that data provider has right signature, check that it returns at least one value, etc

if users will see some generic metamodel exception, they will think, that there is a bug in ceylon.test, I think its better to help users discover their faults

[lucaswerkmeister]

But I don’t agree with those verifications. To me, it seems like both your ArgumentResolver and ArgumentsProvider are mostly the same thing: they’re both based on the notion that parameters should get their arguments independently, based on annotations. With the ArgumentsProvider verifications you’re proposing, I don’t see what other ArgumentResolver implementations would even make sense.

I think there should also be the possibility to write ArgumentsProviders that work completely differently. That don’t use parameter annotations at all, and only work on the entire parameter list, not on individual parameters. For that, your verifications are invalid.

That’s why I think that your current ArgumentResolver and ArgumentsProvider should be merged into a single interface: take a list of parameters, provide a stream of parameter lists. The default implementation would take data from annotations on the individual parameters, and could also verify that all parameters have an annotation (though I think that defaulted parameters without data can be useful, see above for my ceylon.formatter example), that the data provider returns at least one element, and that it returns elements of the right type (though I think that’s unnecessary – AFAIK the error message from invoke() is not cryptic at all; we could of course still catch the metamodel exception and “prettify” it).

Other ArgumentsProvider implementations could then act completely differently, without those restrictions. For example, I might write an ArgumentsProvider for my ceylon.ast tests that just searches for values of the right type in the module (to construct an Invocation you need a Primary, e.g. a BaseExpression; to construct the BaseExpression you need a MemberNameWithTypeArguments; to construct that you need an LIdentifier; oh look, the lidentifier tests define some LIdentifiers, let’s take one of those). I’ve currently done essentially that manually (building up ASTs from other tests’ nodes), but I think that could also be done automatically.

An example for an ArgumentsProvider that provides an argument list as a whole, and can’t operate on individual parameters: you write a system that works with public key cryptography, and write tests for it. Most of the tests need a key pair: one parameter for the public key, and one for the private key. The ArgumentsProvider would calculate a couple of key pairs and return an argument list for each. That’s not possible if each parameter has to be considered individually. (Of course, the test functions could wrap their parameters in a tuple, but that’s ugly.)

On Tue, Dec 08, 2015 at 01:58:37AM -0800, Tomas Hradec wrote:

eg. check that all parameters have assigned arg.provider, check that data provider has right signature, check that it returns at least one value, etc

if users will see some generic metamodel exception, they will think, that there is a bug in ceylon.test, I think its better to help users discover their faults

---

Reply to this email directly or view it on GitHub: https://github.com/ceylon/ceylon-sdk/issues/159#issuecomment-162836554

[thradec]

Well, I can see sometimes differences in naming, what we use, but it seems to me, that the goals are some. So I would recommend to wait on more polished version, which will be soon hopefully ;-), and we can discuss details there.

[thradec]

Polished version pushed into 159-parameterized-tests branch @ad8f989, let me know if there is missing any interesting use case, or whatever ;-)

[lucaswerkmeister]

Sorry, but I dislike parts of it so much that it motivated me to get off my lazy ass and implement my own vision :D see 0f9c31539577f7dbc7d229ad5c69779cc5713fb0. With this, I can run the following parameterized tests: https://gist.github.com/lucaswerkmeister/1be334f4ca140ac051d2

Important to me:

• ArgumentProvider and ArgumentListProvider are two independent, orthogonal concepts. ArgumentProvider provides arguments for a single parameter. ArgumentListProvider provides argument lists for a single test function, potentially (but not necessarily) using ArgumentProviders.
• Default ArgumentListProvider combines ArgumentProviders from annotations, as previously. Any annotation class may satisfy ArgumentProvider. Default annotation given in ceylon.test is argumentProvider (previously called dataProvider), which takes a reference to a toplevel value, as before.
• ArgumentListProviders can be specified via annotation on the test function, or via parameter to the testSuite annotation. (Not sure if this one’s staying in, see below.)
• ArgumentListProvider is not called ArgumentsProvider, to avoid confusion. In the code (and in my mind), arguments now always refers to multiple arguments (for the same parameter), and argumentList is used to refer to an argument list. An argument list is never called “arguments”.

Not important to me:

• How multiple test runs for a function are grouped. I stuck with TestExecutor children as before, since it seems to be the simplest version to me, and I don’t understand this part of ceylon.test too well. If you want to instantiate executors or descriptions lazily, fine by me.

Open problems with my version:

• testSuite’s second parameter, to specify the ArgumentListProvider, cannot be defaulted because of a backend bug that doesn’t permit meta literal default arguments (TODO report that bug). This is of course unacceptable; if the bug doesn’t get fixed in time, we’ll have to find a different solution. (I’d like to have some way to avoid having to specify an argumentListProvider on every test function, but I guess we could release without it. Not a blocker.)
• I don’t understand testSuite. It seems that each test is run once outside of the test suite (with default argument list provider), and once within the test suite (with argument list provider specified by test suite). This of course doesn’t work if the test function needs special arguments from its special argument list provider. But even indepentently of that: what’s the point of testSuite if every function in the suite must be annotated test, and will then be run outside of and inside the test suite?

What I mainly dislike about your version is that your ArgumentsProvider does two things which are in my opinion completely independent, and in fact that shows in your code too: your uses of ArgumentsProvider in valuesFromFunctionArgProvider and calculateArgVariants are completely different – even the expected return type of values() is different (and the way that valuesFromFunctionArgProvider triple-interprets the result is also icky IMO). I don’t know how to say this without sounding like “my design is so much better than yours” :/ … but please, look at my ArgumentProvider and ArgumentListProvider. I think it’s so much more elegant if they’re separated.

[thradec]

No problem, this is important feature, so it should be think through all sides ;-).

In fact, in the beginning I had two independent interfaces (ArgumentProvider/ArgumentListProvider) as you too. Then I realised, that return type of ArgumentProvider is not always {Anything*}, because for DI or mocking there is just one returned value. So I changed it to Anything, and because it can return whatever, it can takes the role of ArgumentListProvider also. The only disadvantage are 2-3 if statements in ArgumentResolver.

And, from user perspective, it doesn't bring any advantage, to keep it separately. He wants simple way, how to express, where argument values are provided. We are not able to warrant type safety anyway.

(just the reasoning behind)

I think, I don't mind to split it again, indeed its more clean (from ArgumentResolver PoV), and I can keep simplicity for user by satisfying both interfaces (for cases where it make sense).

[thradec]

Not important to me:

How multiple test runs for a function are grouped. I stuck with TestExecutor children as before, since it seems to be the simplest version to me, and I don’t understand this part of ceylon.test too well. If you want to instantiate executors or descriptions lazily, fine by me

it's crucial to postpone arguments resolving as late as possible (there is always some @jvasileff ;-) who wants to run 100k tests)

[thradec]

IDE support added in ceylon/ceylon-ide-eclipse@4073017 and c06672e.