Various improvements for TestCases decorator

[mwgamble]

It appears that there isn't any documentation for the TestCases decorator added by #414.

Additionally, after inspecting the code it appears that it doesn't handle iterators (especially generators) very well. Instead of manually unrolling the iterator and then running all of the test cases, it would be ideal if it ran the tests one at a time. This seems more in line with the expectation of using an iterator, especially if memory usage is a concern.

[jamesadarich]

Great points thanks for raising this one. I was looking additionally at improvements here to make it better for property testing.

If you're able to send a pr around this it would be greatly appreciated. Otherwise happy to look at this :)

[jamesadarich]

The improvements mentioned here will be covered in #678 which will introduce a new decorator to avoid confusion with TestCase and a more consistent way of writing these tests. As a result TestCases will be deprecated in 4.0.0 to be removed in 5.0.0

[mwgamble]

I chose Alsatian as the test framework for my recent projects specifically for two reasons:

1. It's typescript-native
2. It has a built-in way to parameterise tests

I wrote a lot of tests to using the TestCases decorator. Hearing that support for this is slated to be removed entirely with no suitable alternative is incredibly disappointing. I'm completely in favour of the TestProperties decorator as an alternative (property-based testing is awesome), but there are plenty of things where hard-coded lists of test cases created by the test author are the only option.

[mwgamble]

See here for some references:

https://github.com/cashmoneyjs/cashmoney/tree/master/tests
https://github.com/cashmoneyjs/number/tree/master/tests

[jamesadarich]

Thanks so much for this great feedback :)

The goal of this replacement is to encourage better test writing without losing functionality.

The problems I see with TestCases at the moment are:

• it's so close to the existing TestCase decorator but behaves quite differently
• bringing everything into memory immediately is not ideal
• for a small set of cases using multiple TestCase decorators is likely more readable
• an array of arrays is difficult to read and comprehend

We will be considering alternatives in the design, for example

@TestProperties(fromArrays(
    [1, 2, 3],
    [4, 5, 6]
))

The goal being to have an explicit way of writing these tests so it's clear they are different, are more readable and we can start applying specific logic in these scenarios e.g. all arrays must be the same length.

I'd love to get your input into this design or perhaps some alternatives if you have any ideas since you are using the existing solution quite heavily :)

[mwgamble]

I agree that the name TestCases isn't great. How about something like TestCaseGenerator? It could still accept any iterator (or a function that returns an iterator) like TestCases does now, but instead of iterating over the input itself (which brings all cases into memory immediately), it should iterate over them one-by-one, running the test once after each iteration of the input.

You could even consider TestCase as a special case of TestCaseGenerator (that might even use TestCaseGenerator under the hood somehow) that just has a single test case.

[jamesadarich]

For me I prefer the use of TestProperties as noted above as it unites iterables without having to have a lot of branches within that decorator and we can use composition to create reusable more specific decorators outside of the framework. On top of this I find it clearer what the intention of this decorator is.

In terms of composition this means you can still create something more specific to your needs if necessary but I wanted to avoid doing this in the framework as TestProperties is likely to handle a lot of different scenarios.

In this specific case I`d expect something like

const TestCaseGenerator = (...cases: Array<any>) => TestProperties(fromArray(...cases));

Alternatively we could curry these functions to make this process even easier?

[mwgamble]

What would that look like from an end-user perspective?

[jamesadarich]

So using the snippet I noted above you should write the following

@TestCaseGenerator([
    [ "arg one", 1 ],
    [ "arg two", 2 ]
])
public testWithArrayArgs(stringArg: string, numberArg: number) {
    // test code
}

TestCaseGenerator is probably not the best name but the goal is to enable naming that makes sense and to build up things from the user's perspective e.g.

const AdminRolesCases = TestProperties(fromArray([ Role.Admin, Role.UserAdmin, Role.SuperAdmin ]));

and used...

@AdminRolesCases
public adminRolesDoSomethingSpecific(adminRole: Role) {
    // check admins do something
}

[mwgamble]

But what about handling for the current situation, where if TestCases is passed a function (or static class method), it will call it and store the result of calling that function?

[jamesadarich]

You should be able to call that function e.g. @TestProperties(buildArguments()) and again if it's returning an array this can be composed @TestProperties(fromArray(buildArguments())). :)

[mwgamble]

The original point of me opening this issue was to not have to do that at all.

Right now, even if my function is a generator the TestCases decorator unwinds the generator (or any iterator for that matter) and stores all the generated test cases in memory before any tests are run. This is horribly inefficient - I've actually had cases where I've had too many test cases and have run out of memory before tests start getting run. The proposed solution - the fromArray helper function - doesn't really solve this issue at all.

The only reason I can see for doing things this way is because the test framework wants to know how many tests there are before it starts running tests. Is there a reason for this? Otherwise, what's wrong with just modifying how TestCases works so as to not fully unwind iterators before it starts running tests?

[jamesadarich]

I think there is a bit of confusion here. We're trying to do the same thing but in the design for the replacement of TestCases there will still be the option to use existing functionality.

As a summary of the design so far TestProperties will replace all use cases of TestCases with additional functionality of iterating over the internal iterator (this will be forced to generator) as to not bring all cases into memory initially.

In order to converge all use cases to a single implementation (generator) we need some helper functions to do that e.g. fromArray which will simply convert the array to a generator in the form that TestProperties is expecting to be able to run test cases. The goal will be to encourage usage of generators by default by providing functions to build these cases properly without having to type every case and also them not being in memory in an array e.g.

@TestProperties(numbers.from(0.05).to(1000.05).step(0.5), randomStrings.ofLength(8))

The above would build the required generators in order to run a lot of tests without the pain of writing them out or consuming a lot of memory. The additional benefit of this design we see is composability as we want people to create their own decorators that are useful in their domain. as mentioned above we can write

const MyUsefulDecorator = TestProperties(numbers.from(0.05).to(1000.05).step(0.5), randomStrings.ofLength(8));

The above can then be reused again and again and even updated without having to change all the tests it affects. We also get a third potential benefit of a small performance increase / code simplification as we don't need to do any runtime type checking in order to implement different sorts of iterator.

All I was trying to highlight in previous posts is the existing functionality will still exist in another form but in order to bring more benefits the syntax will be slightly different. How does that sound?

[mwgamble]

Thank you for taking the time to lay everything out again in full, I appreciate it. It makes a lot more sense now and definitely looks like the right way forward 👍

[jamesadarich]

No worries thanks for your patience. When we've got to a design we're happy with I'll update you with what things are looking like as it would be good to get your input and feedback on the design :)

[jamesadarich]

But this should be pretty close to what's already been discussed we're just ironing out details.