Skip to content

Commit

Permalink
jest: Prepare for eslint-plugin-jest upgrade.
Browse files Browse the repository at this point in the history
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] zulip#4235 (comment)
  • Loading branch information
chrisbobbe committed Sep 17, 2020
1 parent 88350af commit 15c14ae
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 29 deletions.
12 changes: 12 additions & 0 deletions .eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 5 additions & 8 deletions src/__tests__/sentry-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
3 changes: 3 additions & 0 deletions src/emoji/__tests__/data-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
37 changes: 16 additions & 21 deletions src/message/__tests__/fetchActions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand All @@ -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);
});
});

Expand Down
4 changes: 4 additions & 0 deletions src/utils/__tests__/internalLinks-test.js
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down

0 comments on commit 15c14ae

Please sign in to comment.