search

thehyve / react-select-checked

A React select component based on JedWatson/React-Select with checkmarks on selected options.
GNU General Public License v3.0
9 stars 5 forks source link

Extract sync/async decorator in unit tests #20

Closed fedde-s closed 6 years ago

fedde-s commented 6 years ago

Moving this parametrisation logic to a separate function should make the tests more readable, by putting the parameters closer to the actual test and reducing the level of nesting.

It has the added effect of changing the test output to look something like this: Console screenshot that shows each test twice, prefixed with a line that says either with ‘async={ false }’ or with ‘async={ true }’

Listing the two variants of the same test adjacently, rather than grouped with all other (a)synchronous versions of tests. It might be a matter of preference, but I think that this version could scale better if the the number of tests grows.

fedde-s commented 6 years ago

This is not urgent and does not affect runtime functionality in any way, and it might be easier to review after #19 introduces more tests.

fedde-s commented 6 years ago

The main reason for this refactoring is that I feel like it should be easy to see what an individual test case was intended to do, when it starts failing. Listing the ‘decorator’ function right above every test case for which it runs, rather than at the top of the test test suite, should help there.

In addition, separating the logic from the test suite should be more flexible if the structure of the test suite changes over time.

fedde-s commented 6 years ago

@rnugraha: done.

fedde-s commented 6 years ago

The level of nesting changed for the tests, so if you want to review the diff on GitHub it might be useful to check ‘hide whitespace changes’ in the top-right corner (diff settings).