From 15c14aebfea1fd60e3636bb9f85e87549f4a5449 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 20 Aug 2020 21:43:56 -0700 Subject: [PATCH] jest: Prepare for `eslint-plugin-jest` upgrade. A few occurrences of these rules will newly be flagged with the upcoming `eslint-plugin-jest` upgrade: jest/expect-expect: https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/expect-expect.md jest/no-try-expect https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-try-expect.md jest/no-standalone-expect https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-standalone-expect.md Adapt to the first two and disable the third, which seems unhelpful, as noted in code comments. It did, however, lead to an improvement in `sentry-test`, where we move some `expect`s from `beforeEach` and `afterEach` into the tests themselves, since logically they're part of the tests themselves [1]. [1] https://github.com/zulip/zulip-mobile/pull/4235#discussion_r489976746 --- .eslintrc.yaml | 12 +++++++ src/__tests__/sentry-test.js | 13 +++----- src/emoji/__tests__/data-test.js | 3 ++ src/message/__tests__/fetchActions-test.js | 37 ++++++++++------------ src/utils/__tests__/internalLinks-test.js | 4 +++ 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 7c66ae42e13..d5eaee0d21b 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -169,6 +169,18 @@ rules: # see jest-community/eslint-plugin-jest#203 (rule might be removed?) jest/valid-describe: off + # The docs for this rule [1] say that placing an `expect` call + # outside a `test` or `it` block (e.g., in a `beforeEach` or + # `afterEach`) means that the assertion isn't executed. Empirically, + # this seems just wrong [2], and there are a few places where we + # want to call `expect` in a `beforeEach` or `afterEach` to assert + # that multiple tests in the same file aren't interfering with each + # other by leaving bits of state lying around. + # + # [1] https://github.com/jest-community/eslint-plugin-jest/blob/master/docs/rules/no-standalone-expect.md + # [2] https://github.com/zulip/zulip-mobile/pull/4235#discussion_r489984717 + jest/no-standalone-expect: off + # React react/jsx-filename-extension: off # Like RN upstream, we call the files *.js. react/destructuring-assignment: off # The opposite is often much cleaner. diff --git a/src/__tests__/sentry-test.js b/src/__tests__/sentry-test.js index 03fd6377365..a74cb0fb089 100644 --- a/src/__tests__/sentry-test.js +++ b/src/__tests__/sentry-test.js @@ -7,28 +7,25 @@ describe('sentry', () => { describe('is usable without initialization', () => { // Sentry shouldn't be active at all in debug mode -- certainly not while // we're specifically testing that its entry points can be used while it's - // uninitialized. - beforeEach(() => { - expect(isSentryActive()).toBeFalse(); - }); - - afterEach(() => { - expect(isSentryActive()).toBeFalse(); - }); + // uninitialized. These tests assert that. test('breadcrumbs', () => { + expect(isSentryActive()).toBeFalse(); Sentry.addBreadcrumb({ message: 'test message', level: Sentry.Severity.Debug, }); + expect(isSentryActive()).toBeFalse(); }); test('exception reporting', () => { + expect(isSentryActive()).toBeFalse(); // The text here is intended to prevent some hypothetical future reader of // Sentry event logs dismissing the error as harmless expected noise, in // case Sentry is somehow actually initialized at this point. const err = new Error('Jest test error; should not result in a Sentry event'); Sentry.captureException(err); + expect(isSentryActive()).toBeFalse(); }); }); }); diff --git a/src/emoji/__tests__/data-test.js b/src/emoji/__tests__/data-test.js index 1033a97c187..44a622f56dc 100644 --- a/src/emoji/__tests__/data-test.js +++ b/src/emoji/__tests__/data-test.js @@ -7,6 +7,9 @@ import { codeToEmojiMap, getFilteredEmojis } from '../data'; /* eslint-disable no-multi-spaces */ describe('codeToEmojiMap', () => { + // Tell Jest to recognize `check` as a helper function that runs + // assertions. + /* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "check"] }] */ const check = (name, string1, string2) => { expect(string1).toEqual(string2); expect(codeToEmojiMap[name]).toEqual(string1); diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 97797d3f3b7..ec9113b97fc 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -202,15 +202,14 @@ describe('fetchActions', () => { }; fetch.mockResponseSuccess(JSON.stringify(response)); - expect.assertions(2); - try { - await store.dispatch( + await expect( + store.dispatch( fetchMessages({ narrow: HOME_NARROW, anchor: 0, numBefore: 1, numAfter: 1 }), - ); - } catch (e) { + ), + ).rejects.toThrow( // Update this with changes to the message string or error type. - expect(e.message).toBe('Active account not logged in'); - } + new Error('Active account not logged in'), + ); const actions = store.getActions(); @@ -255,15 +254,14 @@ describe('fetchActions', () => { }; fetch.mockResponseSuccess(JSON.stringify(response)); - expect.assertions(2); - try { - await store.dispatch( + await expect( + store.dispatch( fetchMessages({ narrow: HOME_NARROW, anchor: 0, numBefore: 1, numAfter: 1 }), - ); - } catch (e) { + ), + ).rejects.toThrow( // Update this with changes to the error type. - expect(e).toBeInstanceOf(TypeError); - } + TypeError, + ); const actions = store.getActions(); @@ -281,14 +279,11 @@ describe('fetchActions', () => { fetch.mockResponseFailure(fetchError); - expect.assertions(1); - try { - await store.dispatch( + await expect( + store.dispatch( fetchMessages({ narrow: HOME_NARROW, anchor: 0, numBefore: 1, numAfter: 1 }), - ); - } catch (e) { - expect(e).toBe(fetchError); - } + ), + ).rejects.toThrow(fetchError); }); }); diff --git a/src/utils/__tests__/internalLinks-test.js b/src/utils/__tests__/internalLinks-test.js index b61c180e899..8625a2ba94d 100644 --- a/src/utils/__tests__/internalLinks-test.js +++ b/src/utils/__tests__/internalLinks-test.js @@ -1,4 +1,8 @@ /* @flow strict-local */ +// Tell Jest to recognize `expectStream` as a helper function that +// runs assertions. +/* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "expectStream"] }] */ + import type { User } from '../../api/modelTypes'; import { streamNarrow, topicNarrow, groupNarrow, STARRED_NARROW } from '../narrow'; import {