[WIP] Component unit tests using react-testing-library

[idoberko2]

Related Issue:
#309 - react-testing-library attempt

Summary:
Examine the usage of react-testing-library to cover component testing

[idoberko2]

@AWolf81, I started with tests for Sidebar. I'm not sure it's such a good case for examination since this component doesn't have much text in it, and react-text-library is very text-oriented (its powerful tools are getByText and getByLabelText by which you can query the DOM, similar to the way the user interacts with the UI). Please have a look when you have time and I will try to add tests for another component.

[AWolf81]

@idoberko2 Thanks for adding the Sidebar test.
I've created that test for enzyme as well. I'll open a PR in a minute so it's easier to compare.

It looks OK but I'm getting an error message in every test - not sure why. It looks like following:

TypeError: Cannot read property 'now' of undefined
      at Object.<anonymous> (node_modules/wait-for-expect/lib/index.js:16:21)
      at Object.<anonymous> (node_modules/dom-testing-library/dist/wait.js:10:45)
      at Object.<anonymous> (node_modules/dom-testing-library/dist/index.js:76:13)

Seems like a configuration issue but I couldn't find the cause yet.

[melanieseltzer]

getByText(/Your new project was just added.*/);

Is this supposed to be here? Is the parameter supposed to be in quotes?

[AWolf81]

It looks a bit like a comment but it's a regex and there is more text in that element. So it's OK like it is.
Maybe the name of the constant shoud be introductionBlurb so it's more clear that getByText is querying it.

[melanieseltzer]

Ah, gotcha :-) Yeah that works, or maybe just a comment above it explaining the regex (for people like me who see broken comments... 😅)

[melanieseltzer]

Should the test files have this at the top? cc @AWolf81

/* eslint-disable flowtype/require-valid-file-annotation */

[AWolf81]

I'm not sure. Do you want to avoid flow check for test files? I think // @flow is just missing here.

I think we should also use flow in our tests or maybe we should ignore them in the flow configuration if we don't need type checking.

[melanieseltzer]

Looks like only platform.service.test.js and migrations.test.js have // @flow, rest are disabled. Maybe using Flow in the test files is not necessary?

[melanieseltzer]

I'm getting the Cannot read property 'now' of undefined errors as well. Can't seem to find anything concrete on Google. Maybe it's something to do with testEnvironment?

From skimming over the code it seems like react-testing-library is much less verbose than Enzyme which I quite like.

[idoberko2]

@AWolf81, @melanieseltzer, so what do you say? Should we drop react-testing-lib and stick with Enzyme? Do you feel like it's worth another shot?

[AWolf81]

@idoberko2 I think we should stick to Enzyme as there are more examples for tests, no issues in the setup and from the tests we've wrote there is no big difference between the two testing libs.
So I think it's better to use the more mature library.

Some other opinions to Testing-lib vs enzyme can be found in this DEV discussion.

[melanieseltzer]

I'm going to agree and say let's stick with Enzyme for now :-)

[AWolf81]

We can close this for now as the PR for Enzyme will be merged soon. Just waiting for the feedback from @idoberko2 to check if the Enzyme setup & the first tests are OK.