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

Test independance violation #22

Open
Klemo1997 opened this issue Sep 8, 2021 · 3 comments
Open

Test independance violation #22

Klemo1997 opened this issue Sep 8, 2021 · 3 comments

Comments

@Klemo1997
Copy link

Klemo1997 commented Sep 8, 2021

I just found a dependency between tests in tag cucumber in files:

  • MenuButtons.test.js
  • sharingSagas.test.js

There is a missing reset of socketSpy variable, which should be set to default state after each test or else it can possibly change the behavior of tests.

Fix is as simple as adding one line into both files:

// MenuButtons.test.js

describe('MenuButtons', () => {
  ...
  describe('sharing button', () => {
    ...
    afterEach(() => {
      ...
      // Add this line
      socketSpy = undefined;
    });
    ...
  });
  ...
});
// sharingSagas.test.js

describe('sharingSaga', () => {
  ...
  afterEach(() => {
    ...
    // Add this line
    socketSpy = undefined;
  });
  ...
});

This unfornately causes the breakage of test dispatches an action of STOP_SHARING when stop sharing is clicked in MenuButtons.test.js due to failure of function notifySocketOpened where the socketSpy is undefined and it is trying to reach its onopen callback property.

Testcase works because socketSpy is set previously in test dispatches an action of START_SHARING when start sharing is clicked and just skipping this test causes test case failure as well.

Solution would be to somehow call startSharing saga that creates WebSocket and therefore sets the onopen callback property . Easiest solution appears to be just adding this line to test:

// sharingSagas.js
...
    it('dispatches an action of STOP_SHARING when stop sharing is clicked', async () => {
      const store = renderWithStore(<MenuButtons />);
      // Add this line
      click(button('Start sharing'));
      store.dispatch({ type: 'STARTED_SHARING' });
      await notifySocketOpened();
      click(button('Stop sharing'));
      return expectRedux(store)
        .toDispatchAnAction()
        .matching({ type: 'STOP_SHARING' });
    });

Hope it helps.

@Klemo1997
Copy link
Author

Klemo1997 commented Sep 11, 2021

Update:
I recently found a better solution in later chapters in the book, particularly in commit: 393a455 added in tag is-sharing

This is how the updated test looks there

it('dispatches an action of STOP_SHARING when stop sharing is clicked', async () => {
  const store = renderWithStore(<MenuButtons />);
  store.dispatch({ type: 'START_SHARING' });
  await notifySocketOpened();
  store.dispatch({ type: 'STARTED_SHARING' });
  click(button('Stop sharing'));
  return expectRedux(store)
    .toDispatchAnAction()
    .matching({ type: 'STOP_SHARING' });
});

@dirv
Copy link
Collaborator

dirv commented Sep 17, 2021

Thanks @Klemo1997 - this is really helpful. I'm going to take some time today to go through the code again and get an update sent through to Packt. From what you're saying it sounds as if I realised the problem later on in the book but never went back to fix the original source 🙉

@dirv
Copy link
Collaborator

dirv commented Oct 4, 2021

@Klemo1997 I've been thinking this over and I think your first solution is a great one, so I'm going to update the book content to include a discussion on this.

We value test independence highly, and as you've found the tests are clearly not independent at this point. And even though it can be fixed by ensuring a test is written to properly set up its own state correctly, in this case I didn't do that, because it's so easy to forget! The afterEach protects us from that potential trap.

Thanks again for reporting this! 🙏

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

No branches or pull requests

2 participants