Simplified and improved test-cases for ReactMultiChildText-test

[syranide]

Test-cases dissected from #1989

Easier overview of tests and much more complete testing of all possible edge-cases.

[sophiebits]

I like this. Can we add assertions for what the empty string renders to?

[syranide]

@spicyj Ah yes, will add that.

[syranide]

@spicyj Added. Also separated out "already tested test cases" to lessen the impact of O(n^2) and to avoid them being tested against themselves for every "list of tests".

There are probably a bunch of improvements that could be made, but perhaps this is a better starting point than what we have today (which seemed quite unmaintainable, especially when extended to adequately cover merging of adjacent strings).

[sophiebits]

It's hard for me to follow what's going on now. I prefer the simple O(n^2) solution which will be plenty fast as long as we don't have thousands of different tests here, which we won't.

[syranide]

@spicyj 40_40 = 1600 and we're doing actual "rendering" and updating, 1ms each and we're suddenly taking 1.6s for only one of the handful of tests there. 40_30 isn't really better though and perhaps not worth the complication.

But the idea with splitting up as I did was simply to avoid the test-cases in the first argument from being tested against themselves, again, for every separate test. So it's the exact same thing as before (and looks kind of the same), it's just an optimization, although that doesn't necessarily make it easily understood... :)

[sophiebits]

Well, I am fine with combining them into one test if that helps…

[syranide]

@spicyj Merged them all into one test, apparently it was a lot faster than I was led to believe (I imagined it becoming significantly slower as I added more, apparently not), good catch.

[sophiebits]

I think this looks good to me now, thanks.

[chenglou]

@syranide 80 chars please, this triggers the linter =)

[syranide]

@chenglou Can do, although I was under the impression that char limits don't "really" apply wholesale to tests (many existing tests break 80 chars and other conventions). I don't mind doing it if you insist, but I feel that it would overall reduce the readability (as it's also using pairs of arguments to form a "test-case").

But again, I don't mind if you insist. :)

[chenglou]

It's fine!