Tests refactoring checklist

[Hypnosphi]

Initially, I created a project, but then I realised that it's not commentable so maybe an umbrella issue with checklist would be more useful, so I duplicate it as such.

@joshwcomeau has mentioned that he would like to have some improvements. So I decided to make some kind of checklist for what has to be done. Feel free to add anything that you think would be nice to do.

Definitely needed:

• Isolate the tests from each other, i.e. make all the setup before each test so that the state is reset Isolate testcases from each other #166
• Add tests for appear / enter / leave animations
• Do something with tests stability (see false positives like that: https://travis-ci.org/joshwcomeau/react-flip-move/jobs/236306660) Increase CI tests stability by replacing window.requestAnimationFrame #175
• Add tests for runtime props conversion and checks Add tests for runtime props conversion and checks #174
• Use sandboxed sinon for easier resetting/restoring

Maybe needed:

• Add simple unit tests for helpers — not really needed, they are fully covered by other tests
• Use some code coverage tool like istanbul Add karma-coverage reporter #172
• Use enzyme for easier react output manipulation Use enzyme for testing React output #169
• Use sinon fake timers instead of setTimeout

[joshwcomeau]

Awesome!

Yeah, so I originally wrote this module when I was completely inexperienced with React tests. I still don't have a ton of experience with it, but I can certainly see how bad they are now.

The easiest and most crucial win is just separating each test, as you say, so that each test mounts its own DOM, performs the test, and then unmounts it.

The false-positives thing has been very tricky in my experience - Working locally, the timing is very accurate, but on CI the timing is far less predictable. An animation set to 500ms will usually be within a few ms locally, but can take 1000-1500ms on travis, for reasons that aren't super clear to me.

The "maybe needed" stuff all sounds good to me - the helpers really should be unit-tested, and I like Istanbul/Enzyme.

[Hypnosphi]

I've added one more point: "use sinon fake timers instead of setTimeout". To do that, we also need to use some stub for requestAnimationFrame like this one, or write one ourselves as react-motion did.

Upd: OK, it's not really achievable as we're using CSS transitions

[Hypnosphi]

The false-positives thing has been very tricky in my experience - Working locally, the timing is very accurate, but on CI the timing is far less predictable. An animation set to 500ms will usually be within a few ms locally, but can take 1000-1500ms on travis, for reasons that aren't super clear to me.

Maybe the problem is that we don't take two skipped frames into account?

[joshwcomeau]

Maybe the problem is that we don't take two skipped frames into account?

Yeah, maybe if CI only runs at 1-2fps, it could explain it.

[tobilen]

• regarding " Use sandboxed sinon for easier resetting/restoring", have you given jests mock functionality a look? we've replaced also most all of our sandboxes with the mocking toolset provided by jest.

• regarding "Add tests for appear / enter / leave animations" we could look for classes mapped to animation ("--anim-start" etc.) maybe? i mean, short of that you need something like cypress.io for actual e2e testing i wager.

• adding tests for high pressure situations (500 incoming elements, 200 leaving, 600incoming all incoming in short distance). it'll most likely mess up sorting. we'd then have a red to work up from. i'll probably build something in that regard this weekend