Skip to content

Commit

Permalink
fix(frontend): fix frontend test flakiness (#1162)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
zwliew authored Apr 29, 2021
1 parent de89b49 commit 04c49a2
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 8 deletions.
1 change: 0 additions & 1 deletion amplify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ frontend:
build:
commands:
- npm run build
- CI=true npm run test
- npm run upload-source-map
artifacts:
# IMPORTANT - Please verify your build output directory
Expand Down
15 changes: 8 additions & 7 deletions frontend/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { InitializeOptions } from 'react-ga'
// Re-export these later on as constants
let gaInitializeOptions: InitializeOptions
let gaTrackingId: string
let announcementActive: string

/**
* Configs differentiated between environments
Expand All @@ -17,6 +18,9 @@ if (process.env.NODE_ENV === 'test') {
gaInitializeOptions = {
testMode: true,
}

// Disable announcements in test environments
announcementActive = 'false'
} else {
/**
* React env vars are used for injecting variables at build time
Expand Down Expand Up @@ -44,6 +48,9 @@ if (process.env.NODE_ENV === 'test') {
gaInitializeOptions = {
debug: false, // Set to true only on development
}

// `REACT_APP_ANNOUNCEMENT_ACTIVE` must be set to `true` in Amplify if we want the modal to display
announcementActive = process.env.REACT_APP_ANNOUNCEMENT_ACTIVE as string
}

/**
Expand Down Expand Up @@ -86,16 +93,10 @@ export const SENTRY_ENVIRONMENT =
(process.env.REACT_APP_SENTRY_ENVIRONMENT as string) || 'development'
export const INFO_BANNER = process.env.REACT_APP_INFO_BANNER as string

// `REACT_APP_ANNOUNCEMENT_ACTIVE` must be set to `true` in Amplify if we want the modal to display
function getAnnouncementActive() {
const envVar = process.env.REACT_APP_ANNOUNCEMENT_ACTIVE as string
return envVar === 'true'
}

// Feature Launch Announcements
// If `isActive` is false, the modal will not proc for ANY user
export const ANNOUNCEMENT: Record<string, any> = {
isActive: getAnnouncementActive(),
isActive: announcementActive === 'true',
title: t`announcement.title`,
subtext: t`announcement.subtext`,
mediaUrl: t`announcement.mediaUrl`,
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import { Crypto } from '@peculiar/webcrypto'
import 'locales' // Locales necessary for I18nProvider
import { server } from './test-utils'

// Some tests take longer than the default 5s to run.
// Hence, allow the test timeout to be configured.
const TEST_TIMEOUT = +(process.env.REACT_APP_TEST_TIMEOUT ?? 20000)
jest.setTimeout(TEST_TIMEOUT)

// Mock WebCrypto APIs
// Redeclare the type of `global` as it does not include the `crypto` prop
interface Global extends NodeJS.Global {
Expand Down
9 changes: 9 additions & 0 deletions frontend/src/test-utils/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ function mockSettingsApis(state: State) {
})
)
}),
rest.put('/settings/announcement-version', (req, res, ctx) => {
const { announcement_version } = req.body as {
announcement_version?: string
}
if (!announcement_version) {
return res(ctx.status(400))
}
return res(ctx.status(200))
}),
rest.get('/settings/email/from', (_req, res, ctx) => {
return res(
ctx.status(200),
Expand Down

0 comments on commit 04c49a2

Please sign in to comment.