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

Flaky frontend tests #1161

Closed
zwliew opened this issue Apr 28, 2021 · 2 comments · Fixed by #1162
Closed

Flaky frontend tests #1161

zwliew opened this issue Apr 28, 2021 · 2 comments · Fixed by #1162
Labels
bug Something isn't working

Comments

@zwliew
Copy link
Contributor

zwliew commented Apr 28, 2021

Describe the bug
Some of the integration tests fail sporadically with the following error:

thrown: "Exceeded timeout of 5000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

Specifically, this was experienced commonly with src/components/dashboard/tests/integration/protected-email.test.tsx.

To Reproduce
Steps to reproduce the behavior:

  1. Run npm test. It should fail every now and then.

Expected behaviour
All tests should pass 100% of the time.

Possible solution

  1. Maybe we need to increase the timeout by adding jest.setTimeout(SOME_LONG_TIMEOUT) to setupTests.ts?
  2. Maybe the test are not ending when they should be?
@zwliew zwliew added the bug Something isn't working label Apr 28, 2021
@zwliew
Copy link
Contributor Author

zwliew commented Apr 28, 2021

Braindump

Might have something to do with 381f1cf#diff-ba1169b6c91be2d66b1961f46d2a09a28fd896b90f9dc1cefe791e3d3af68ec0R28

Previously, we weren't making the final getCampaignDetails API call after a campaign is finished sending. Thus, I wrote the integration tests to end immediately after the campaign status is SENT.

Now, with the final API call, it is incorrect for the tests to end immediately. Instead, they should wait for the API call to finish before ending.

@zwliew
Copy link
Contributor Author

zwliew commented Apr 28, 2021

Increasing the test timeout to 10s on a local MBP 16" using jest.setTimeout(10000) seems to fix the failures. (Tested over 10 runs with no cache via CI=true npm test -- --no-cache)

zwliew added a commit that referenced this issue Apr 28, 2021
We were experiencing flaky tests as described in issue #1161.

A timeout of 10s works fine on a local MBP 16". However, given that
the CI servers are likely much weaker, 2x the timeout to be on the safe
side.
zwliew added a commit that referenced this issue Apr 29, 2021
* fix(frontend): increase jest timeout to 20s

We were experiencing flaky tests as described in issue #1161.

A timeout of 10s works fine on a local MBP 16". However, given that
the CI servers are likely much weaker, 2x the timeout to be on the safe
side.

* feat: configure the timeout using an env var

This is so that we can configure the timeout based on the test
environment.

* fix(test): add /settings/announcement-version API mock

* fix: add a dummy API mock for /settings/announcement-version

* fix: disable announcements for all tests

We don't have any tests that test the announcement modal.
Hence, disable it for now.

A longer-term fix would be to add announcement_version to the initial
state of the API mock.

* fix: correct the type of isActive

* chore: don't run tests on Amplify

We only rely on Amplify for deployments, so there isn't a major reason
to run tests there.

The main reason is to block the deployment if a test fails, but any test
failures should've been caught on Travis during development.

If a failure does happen during deployment, we can manually redeploy an
older release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant