Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single expectation tests - Discussion #6

Open
Noriller opened this issue Jun 12, 2022 · 4 comments
Open

Single expectation tests - Discussion #6

Noriller opened this issue Jun 12, 2022 · 4 comments

Comments

@Noriller
Copy link
Contributor

I understand the idea behind it, but sometimes it's not feasible.

When you're testing the frontend, sometimes you need to check multiple things.
So, what comes to mind, would be either nest describe blocks or to group related expectations.

Nesting describe blocks, now that I'm thinking about, can lead to DRY tests, but at the same time it could increase test complexity, since you would have to check multiple levels to understand the context.

A suggestion I could make is to change it to "prefer single expectation tests" and "group related expectations".

@antgonzales
Copy link
Owner

@Noriller that's interesting, do you want to maybe give me an example that might help make it clear when to break down expectations into a group?

@Noriller
Copy link
Contributor Author

After trying the single expectations for a while, there are still a few cases where I would group expectations

  • when checking mocks: how many times and with what they were called
    • no sense in splitting into one expectation on how many times and another to check with what it was called
  • on forms, sometimes an event affects multiple fields in the same way (eg: form clear), so checking the fields is something I group together
    • otherwise, you would need to repeat everything but a word multiple times basically
  • some events toggle/change something, so the checking of something not being in the document while another is, is something I feel like is a "unit"
    • while you could have one for the removal and another to check if it is there, I feel like putting them together makes more sense

there are probably more cases, but I would say to group things that don't make sense to split or that would make you repeat the test name

@antgonzales
Copy link
Owner

I see where you're coming from but I also kind of think these are more guidelines than strict requirements. I would like to critique some of your examples in hopes of thinking about the problem a little differently.

when checking mocks: how many times and with what they were called

In general, I don't like to use mocks because they make it difficult to change APIs. For instance, if you mock fetch() but want to switch to isomorphic-fetch for SSR, it means you have to update the API in production code and your tests. Instead, I like to use mws or nock which offer the ability to ensure that an API was called once:

If we're testing a component, I would probably lean on integration tests by checking what the user sees.

on forms, sometimes an event affects multiple fields in the same way (eg: form clear), so checking the fields is something I group together

That makes sense, but you could also perform a loop on test assertions OR follow the Nest describe blocks for context rule.

it('resets the form', () => {
  const form = document.getElementById('form');
  const formData = new FormData(form);

  const output = document.getElementById('output');

  for (const [key, value] of formData) {
    expect(value).toBe('');
  }
})
describe('<Form />', () => {
  describe('when a customer resets the form', () => {
    ...
  });
});

some events toggle/change something

That would make sense, I see where you're coming from on that one. I would probably use two assertions for a toggle.

Overall, I think the perspective is to keep your tests focused. I wrote this particular rule because I noticed developers checking behavior and checking side-effects in the same test. Having multiple assertions usually indicates a lack of focus. Feel free to open a PR if you think the wording of the guidelines could be improved 👍

@Noriller
Copy link
Contributor Author

I like the use of iteration to check multiple things that should be similar. (totally forgot I could use that!)

But mocks are a necessary evil... In the backend, for example, you usually mock the repository in unit tests, in which case it usually makes more sense to keep the calls and with what is coupled in one test than splitting in multiple.

Another case is that of tables, maybe using iteration could solve that, but I would find it overkill to make tests based on each cell. (Snapshot is a possibility, but personally, I don't like them.)

BTW, I'm using (most of) that: https://dev.to/noriller/a-refreshing-way-to-test-the-frontend-5a6c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants