Proposal: Update cell tests to just test that nothing is thrown

[cannikin]

Right now the cell tests are testing very specific (sometimes the exact) text output from each state (Loading, Empty, Failure, Success). As soon as you change anything in any of these functions, the test fails. I feel like this is not ideal for new developers, or new-to-Redwood developers—a failing test inherently signals to you that you did something "bad" but in this case you just started building out your real functionality. Why should they be "punished" for that?

I propose that the default Cell tests follow the lead of the default Component, Page and Layout tests: just check that no errors are raised. Once you get your component/cell dialed in you can update your test to target specific output, but when you're just building them out the safest thing to check is that no errors are raised:

test('Success renders successfully', async () => {
  expect(() => {
    render(<Success posts={standard().posts} />)
  }).not.toThrow()
})

It still uses the mocks, and passes them to the components for rendering, but the expectation is just that you didn't horrifically break anything.

I think we can agree that having a Page test that tested for the presence of the default "Find me in web/src/pages/HomePage/HomePage.js" since will pass once or, if they don't bother running the suite, never! Users are guaranteed to change it, and break the test, and have to change that as well. So let's not force them to do extra work just to get started in a Cell either.

We're seeing this cause issues specifically during the tutorial—by the time you finish, if you were to run the test suite, you'd get failures in all of your cells. I don't think this is a good first experience for users. I would love, at the end of part 1 of the tutorial, to tell the reader:

By the way, run yarn rw test—you've got a full test suite! Now check out part 2 to learn about writing your own tests...

Before I could start on part 2 of the tutorial I had to update the test suite so that I could start out new users with everything being green. Let's default them to the happy path and then explain to them the virtues of adding tests and keeping them up-to-date with their component code.

/cc @mojombo @RobertBroersma

[RobertBroersma]

100% agree.

Only detail I'd change is render the Cell itself instead of the Success component or as well? Not sure, but somehow I think it would be good to have this throwing test for the integration of the Cell as well. Wdyt?

[cannikin]

Only detail I'd change is render the Cell itself instead of the Success component or as well? Not sure, but somehow I think it would be good to have this throwing test for the integration of the Cell as well. Wdyt?

What does an expect look like in this case? Does it need to somehow wait for the final render state of ? Would it work with the standard() mock stuff that's already in place, or would we have to have MSW involved?

[RobertBroersma]

Sorry, I think I'm mixing up some stuff at the moment. What I suggested would indeed require mock setup, and changing the output (i.e. running through the tutorial) would make it fail.

Disregard my comment! Let me try again:

I 100% agree.

[thedavidprice]

For reference, here's the current suggested Cell Test template that's queued up in the Testing Project Board to be implemented:
#629 (comment)

I would love, at the end of part 1 of the tutorial, to tell the reader:
By the way, run yarn rw test—you've got a full test suite! Now check out part 2 to learn about writing your own tests...

^^ I would also love to see this happen.

The journey with tests so far seems to be an attempt to balance:

• starting with something that's meaningful + foundationally instructive
• without going so far as an initial test that's too narrow or brittle

I think Robert's proposed solution in #629 is an attempt to avoid being narrow, e.g. test the Cell and not the specific Failure, Succes, e.g. components. But that might be too broad (doesn't check for Null or Loading, although it could).

I think Rob's proposal to check for "not throwing" hits the right expectation for being meaningful. Maybe, via comments, there could be a quick example or two about how to test for specific content if the user wants to do so?

My suggestion at this point:

• User Roberts new template
  • [edit] with the addition of incorporating mock data from the Cell.mock.js file
• add test for Null and Loading (if possible) states
• only check for "does not throw"
• add an example syntax as a comment about possible improvement step

[cannikin]

That Issue was opened long before @peterp came up with the .mock.js files and standard() stuff... does it even still apply? The idea behind the mock solution we have now was to avoid having to do any kind of manual mocking in every test and just create the data structure you wanted to see in mock.js and it's available everywhere.

[thedavidprice]

does it even still apply?

It does as there's been an ongoing conversation (and PRs) that keep looping back to the Issue. E.g Robert's comment with the template I referenced above came after I updated the current Cell test template to use the mock file. Robert's intent was to focus the testing on the Cell component instead of each specific "state" component within the cell, which seemed like a good evolution. But I'd agree it seems out of sync that he did not include the use of the Cell.mock.js file for the data.

I understand that using the Mock file for the data structure is how Peter intended this to work together. But I missed a point on my bulleted list to be explicit about using the Cell.mock.js for data, which would be a change to Robert's template.

Now edited and added it to the list (sorry I missed it the first time). More clear? (or just more muddy?)

[cannikin]

After discussing with @thedavidprice I'm going to add a comment in here along the lines of "when you're ready to test the actual output of your component..." and then, when ready, also add a link to our testing docs.

[thedavidprice]

Closes #629

[RobertBroersma]

E.g Robert's comment with the template I referenced above came after I updated the current Cell test template to use the mock file.

I think my template was after the update, and briefly before we discussed not writing tests at all.

I think having templates that check for throwing is a good middle ground

[thedavidprice]

🚀

[thedavidprice]

I like it, @cannikin! Moving this forward adds great motivation on my end to wrap up the remaining Testing Project Board items this week.

[cannikin]

Now just gotta get the checks to pass 😬