Skip to content

Enable Jest ESLint plugin with recommended config #1728

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions eslint.config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from 'node:path';
import { globalIgnores } from 'eslint/config';
import jest from 'eslint-plugin-jest';
import prettierRecommended from 'eslint-plugin-prettier/recommended';
import globals from 'globals';
import tsEslint from 'typescript-eslint';
Expand Down Expand Up @@ -52,10 +53,6 @@ const config = tsEslint.config([
globals: {
...globals.browser,
...globals.node,
...globals.mocha,
...globals.jest,
__DEBUG_SERVER_ERRORS__: true,
__SERVER_ERRORS__: true,
},

parserOptions: {
Expand Down Expand Up @@ -178,6 +175,16 @@ const config = tsEslint.config([
'@typescript-eslint/restrict-template-expressions': 'off',
},
},
{
files: ['node_package/tests/**', '**/*.test.{js,jsx,ts,tsx}'],

extends: [jest.configs['flat/recommended'], jest.configs['flat/style']],

rules: {
// Allows Jest mocks before import
'import/first': 'off',
},
},
// must be the last config in the array
// https://github.com/prettier/eslint-plugin-prettier?tab=readme-ov-file#configuration-new-eslintconfigjs
prettierRecommended,
Expand Down
10 changes: 2 additions & 8 deletions node_package/tests/Authenticity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,22 @@ meta.content = testToken;
document.head.appendChild(meta);

describe('authenticityToken', () => {
expect.assertions(2);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eslint-plugin-jest bans these expect.assertions(...) calls outside actual tests.

The ones in them are allowed but not actually useful, they are good in two cases:

  1. testing async logic with callbacks instead of async/await;
  2. conditional expect including in try/catch, also banned by the plugin.

it('exists in ReactOnRails API', () => {
expect.assertions(1);
expect(typeof ReactOnRails.authenticityToken).toEqual('function');
expect(typeof ReactOnRails.authenticityToken).toBe('function');
});

it('can read Rails CSRF token from <meta>', () => {
expect.assertions(1);
const realToken = ReactOnRails.authenticityToken();
expect(realToken).toEqual(testToken);
});
});

describe('authenticityHeaders', () => {
expect.assertions(2);
it('exists in ReactOnRails API', () => {
expect.assertions(1);
expect(typeof ReactOnRails.authenticityHeaders).toEqual('function');
expect(typeof ReactOnRails.authenticityHeaders).toBe('function');
});

it('returns valid header with CSRF token', () => {
expect.assertions(1);
const realHeader = ReactOnRails.authenticityHeaders();
expect(realHeader).toEqual({ 'X-CSRF-Token': testToken, 'X-Requested-With': 'XMLHttpRequest' });
});
Expand Down
17 changes: 4 additions & 13 deletions node_package/tests/ComponentRegistry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ describe('ComponentRegistry', () => {
});

it('registers and retrieves React function components', () => {
expect.assertions(1);
const C1 = () => <div>HELLO</div>;
ComponentRegistry.register({ C1 });
const actual = ComponentRegistry.get('C1');
Expand All @@ -46,7 +45,6 @@ describe('ComponentRegistry', () => {
});

it('registers and retrieves Render-Function components where property renderFunction is set and zero params', () => {
expect.assertions(1);
const C1 = () => <div>HELLO</div>;
C1.renderFunction = true;
ComponentRegistry.register({ C1 });
Expand All @@ -56,7 +54,6 @@ describe('ComponentRegistry', () => {
});

it('registers and retrieves ES5 class components', () => {
expect.assertions(1);
const C2 = createReactClass({
render() {
return <div> WORLD </div>;
Expand All @@ -69,7 +66,6 @@ describe('ComponentRegistry', () => {
});

it('registers and retrieves ES6 class components', () => {
expect.assertions(1);
class C3 extends React.Component {
render() {
return <div>Wow!</div>;
Expand All @@ -82,7 +78,6 @@ describe('ComponentRegistry', () => {
});

it('registers and retrieves renderers if 3 params', () => {
expect.assertions(1);
const C4 = (a1, a2, a3) => null;
ComponentRegistry.register({ C4 });
const actual = ComponentRegistry.get('C4');
Expand All @@ -95,7 +90,6 @@ describe('ComponentRegistry', () => {
* Thus, tests are cumulative.
*/
it('registers and retrieves multiple components', () => {
expect.assertions(4);
// Plain react stateless functional components
const C5 = () => <div>WHY</div>;
const C6 = () => <div>NOW</div>;
Expand Down Expand Up @@ -127,7 +121,6 @@ describe('ComponentRegistry', () => {
});

it('only detects a renderer function if it has three arguments', () => {
expect.assertions(2);
const C7 = (a1, a2) => null;
const C8 = (a1) => null;
ComponentRegistry.register({ C7 });
Expand All @@ -148,20 +141,17 @@ describe('ComponentRegistry', () => {
});

it('throws error for retrieving unregistered component', () => {
expect.assertions(1);
expect(() => ComponentRegistry.get('foobar')).toThrow(
/Could not find component registered with name foobar/,
);
});

it('throws error for setting null component', () => {
expect.assertions(1);
const C9 = null;
expect(() => ComponentRegistry.register({ C9 })).toThrow(/Called register with null component named C9/);
});

it('retrieves component asynchronously when registered later', async () => {
expect.assertions(1);
const C1 = () => <div>HELLO</div>;
const componentPromise = ComponentRegistry.getOrWaitForComponent('C1');
ComponentRegistry.register({ C1 });
Expand All @@ -175,11 +165,12 @@ describe('ComponentRegistry', () => {
});

it('handles timeout for unregistered components', async () => {
expect.assertions(1);
let error;
try {
await ComponentRegistry.getOrWaitForComponent('NonExistent');
} catch (error) {
expect(error.message).toMatch(/Could not find component/);
} catch (e) {
error = e;
}
expect(error.message).toMatch(/Could not find component/);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoids conditional expect (though expect.assertions will catch the problem if there is no error too, we could just disable the rule here instead).

});
});
13 changes: 1 addition & 12 deletions node_package/tests/ReactOnRails.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ import * as createReactClass from 'create-react-class';
import ReactOnRails from '../src/ReactOnRails.client';

describe('ReactOnRails', () => {
expect.assertions(14);
it('render returns a virtual DOM element for component', () => {
expect.assertions(1);
const R1 = createReactClass({
render() {
return <div> WORLD </div>;
Expand All @@ -25,42 +23,36 @@ describe('ReactOnRails', () => {
document.body.appendChild(root);
ReactOnRails.render('R1', {}, 'root');

expect(document.getElementById('root').textContent).toEqual(' WORLD ');
expect(document.getElementById('root').textContent).toBe(' WORLD ');
});

it('accepts traceTurbolinks as an option true', () => {
ReactOnRails.resetOptions();
expect.assertions(1);
ReactOnRails.setOptions({ traceTurbolinks: true });
const actual = ReactOnRails.option('traceTurbolinks');
expect(actual).toBe(true);
});

it('accepts traceTurbolinks as an option false', () => {
ReactOnRails.resetOptions();
expect.assertions(1);
ReactOnRails.setOptions({ traceTurbolinks: false });
const actual = ReactOnRails.option('traceTurbolinks');
expect(actual).toBe(false);
});

it('not specified has traceTurbolinks as false', () => {
ReactOnRails.resetOptions();
expect.assertions(1);
ReactOnRails.setOptions({});
const actual = ReactOnRails.option('traceTurbolinks');
expect(actual).toBe(false);
});

it('setOptions method throws error for invalid options', () => {
ReactOnRails.resetOptions();
expect.assertions(1);
expect(() => ReactOnRails.setOptions({ foobar: true })).toThrow(/Invalid option/);
});

it('registerStore throws if passed a falsey object (null, undefined, etc)', () => {
expect.assertions(3);

expect(() => ReactOnRails.registerStore(null)).toThrow(/null or undefined/);

expect(() => ReactOnRails.registerStore(undefined)).toThrow(/null or undefined/);
Expand All @@ -69,7 +61,6 @@ describe('ReactOnRails', () => {
});

it('register store and getStoreGenerator allow registration', () => {
expect.assertions(2);
function reducer() {
return {};
}
Expand All @@ -87,7 +78,6 @@ describe('ReactOnRails', () => {
});

it('setStore and getStore', () => {
expect.assertions(2);
function reducer() {
return {};
}
Expand All @@ -109,7 +99,6 @@ describe('ReactOnRails', () => {
});

it('clearHydratedStores', () => {
expect.assertions(2);
function reducer() {
return {};
}
Expand Down
19 changes: 6 additions & 13 deletions node_package/tests/StoreRegistry.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ function storeGenerator2(props) {
return createStore(reducer, props);
}

describe('', () => {
expect.assertions(11);
it('StoreRegistry throws error for registering null or undefined store', () => {
expect.assertions(2);
describe('StoreRegistry', () => {
beforeEach(() => {
StoreRegistry.stores().clear();
});

it('StoreRegistry throws error for registering null or undefined store', () => {
expect(() => StoreRegistry.register({ storeGenerator: null })).toThrow(
/Called ReactOnRails.registerStoreGenerators with a null or undefined as a value/,
);
Expand All @@ -28,15 +29,12 @@ describe('', () => {
});

it('StoreRegistry throws error for retrieving unregistered store', () => {
expect.assertions(1);
StoreRegistry.stores().clear();
expect(() => StoreRegistry.getStore('foobar')).toThrow(
/There are no stores hydrated and you are requesting the store/,
);
});

it('StoreRegistry registers and retrieves Render-Function stores', () => {
expect.assertions(2);
StoreRegistry.register({ storeGenerator, storeGenerator2 });
const actual = StoreRegistry.getStoreGenerator('storeGenerator');
const expected = storeGenerator;
Expand All @@ -46,23 +44,20 @@ describe('', () => {
expect(actual2).toEqual(expected2);
});

it('StoreRegistry throws error for retrieving unregistered store', () => {
expect.assertions(1);
it('StoreRegistry throws error for retrieving unregistered store generator', () => {
expect(() => StoreRegistry.getStoreGenerator('foobar')).toThrow(
/Could not find store generator registered with name foobar\. Registered store generator names include/,
);
});

it('StoreRegistry returns undefined for retrieving unregistered store, passing throwIfMissing = false', () => {
expect.assertions(1);
StoreRegistry.setStore('foobarX', {});
const actual = StoreRegistry.getStore('foobar', false);
const expected = undefined;
expect(actual).toEqual(expected);
});

it('StoreRegistry getStore, setStore', () => {
expect.assertions(1);
const store = storeGenerator({});
StoreRegistry.setStore('storeGenerator', store);
const actual = StoreRegistry.getStore('storeGenerator');
Expand All @@ -71,14 +66,12 @@ describe('', () => {
});

it('StoreRegistry throws error for retrieving unregistered hydrated store', () => {
expect.assertions(1);
expect(() => StoreRegistry.getStore('foobar')).toThrow(
/Could not find hydrated store registered with name foobar\. Registered hydrated store names include/,
);
});

it('StoreRegistry clearHydratedStores', () => {
expect.assertions(2);
StoreRegistry.clearHydratedStores();

const result = storeGenerator({});
Expand Down
18 changes: 2 additions & 16 deletions node_package/tests/buildConsoleReplay.test.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,24 @@
import buildConsoleReplay, { consoleReplay } from '../src/buildConsoleReplay';

describe('consoleReplay', () => {
expect.assertions(8);
it('does not throw an exception if no console.history object', () => {
expect.assertions(1);
expect(() => consoleReplay()).not.toThrow(/Error/);
});

it('returns empty string if no console.history object', () => {
expect.assertions(1);
const actual = consoleReplay();
const expected = '';
expect(actual).toEqual(expected);
});

it('does not throw an exception if console.history.length is zero', () => {
expect.assertions(1);
console.history = [];
expect(() => consoleReplay()).not.toThrow(/Error/);
});

it('returns empty string if no console.history object', () => {
expect.assertions(1);
it('returns empty string if console.history is empty', () => {
console.history = [];
const actual = consoleReplay();
const expected = '';
expect(actual).toEqual(expected);
});

it('replays multiple history messages', () => {
expect.assertions(1);
console.history = [
{ arguments: ['a', 'b'], level: 'log' },
{ arguments: ['c', 'd'], level: 'warn' },
Expand All @@ -40,7 +29,6 @@ describe('consoleReplay', () => {
});

it('replays converts console param objects to JSON', () => {
expect.assertions(1);
console.history = [
{ arguments: ['some message', { a: 1, b: 2 }], level: 'log' },
{ arguments: ['other message', { c: 3, d: 4 }], level: 'warn' },
Expand All @@ -52,8 +40,7 @@ console.warn.apply(console, ["other message","{\\"c\\":3,\\"d\\":4}"]);`;
expect(actual).toEqual(expected);
});

it('replays converts script tag inside of object string to be safe ', () => {
expect.assertions(1);
it('replays converts script tag inside of object string to be safe', () => {
console.history = [
{
arguments: [
Expand All @@ -74,7 +61,6 @@ console.warn.apply(console, ["other message","{\\"c\\":3,\\"d\\":4}"]);`;
});

it('buildConsoleReplay wraps console replay in a script tag', () => {
expect.assertions(1);
console.history = [
{ arguments: ['some message', { a: 1, b: 2 }], level: 'log' },
{ arguments: ['other message', { c: 3, d: 4 }], level: 'warn' },
Expand Down
Loading
Loading