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 / eclipse-archived/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 😉

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 😉

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 eclipse-archived/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.

[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:
#159 (comment)

[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 0f9c315. 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 eclipse-archived/ceylon-ide-eclipse@4073017 and c06672e.

[thradec]

Rebased and merged to master, via facc616 and eclipse-archived/ceylon-ide-eclipse@2c68a85.

Recapitulation:

• arguments values can be provided via annotations, which satisfy contract ArgumentProvider or ArgumentListProvider
• there is currently one implementation of this concept, annotation parameters (another can be easily implemented)
• arguments values are resolved lazily
• before/after callbacks can be parameterised also
• ide/reports shows every test variant independently

Example:

shared {[Integer, Integer]*} fibonnaciNumbers => {[1, 1], [2, 1], [3, 2], [4, 3], [5, 5], [6, 8] ...};

test
parameters(`value fibonnaciNumbers`)
shared void shouldCalculateFibonacciNumber(Integer input, Integer result)
    => assert(fibonacciNumber(input) == result);

I'm going to close this issue, for any improvements or new usecases, please open new issue.

[luolong]

👍 Good work!