Skip to content

CONSOLE-5286 Block snapshot tests via ESLint and migrate existing tests to explicit assertions #16427

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.

Sign up for GitHub

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
cajieh wants to merge 1 commit into
base: main
Choose a base branch
from
Open

CONSOLE-5286 Block snapshot tests via ESLint and migrate existing tests to explicit assertions #16427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion .claude/skills/gen-rtl-test/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,9 @@ it('renders')

### Rule 18: Avoid Snapshot Tests

**DO NOT** use `toMatchSnapshot()`. Snapshot tests are brittle, give false security, and test implementation details.
**DO NOT** use `toMatchSnapshot()`, `toMatchInlineSnapshot()`, or error snapshot matchers. Snapshot tests are brittle, give false security, and test implementation details. Prefer **`toStrictEqual`**, **`toMatchObject`**, or RTL queries on user-visible output.

**Enforcement:** `jest/no-restricted-matchers` from `eslint-plugin-console` **errors** on these matchers for paths matched by `plugin:console/testing-library-tests` (the same `**/*spec*` / `**/__tests__**` globs used for RTL lint).

### Rule 19: Render in Each Test by Default

Expand Down
140 changes: 0 additions & 140 deletions ...d/packages/dev-console/src/utils/__tests__/__snapshots__/shared-submit-utils.spec.ts.snap
View file
Open in desktop

This file was deleted.

139 changes: 131 additions & 8 deletions frontend/packages/dev-console/src/utils/__tests__/shared-submit-utils.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,75 @@ describe('Shared submit utils', () => {
expect(serviceObj.spec.ports[0].port).toEqual(8081);
});

it('should match the previous snapshot created with git import data', () => {
it('should build the expected service object for git import data', () => {
const mockData: GitImportFormData = _.cloneDeep(mockFormData);
const serviceObj = createService(mockData);
expect(serviceObj).toMatchSnapshot();
expect(serviceObj).toStrictEqual({
apiVersion: 'v1',
kind: 'Service',
metadata: {
annotations: {
'app.openshift.io/vcs-ref': '',
'app.openshift.io/vcs-uri': 'https://github.com/test/repo',
'openshift.io/generated-by': 'OpenShiftWebConsole',
},
labels: {
app: 'test-app',
'app.kubernetes.io/component': 'test-app',
'app.kubernetes.io/instance': 'test-app',
'app.kubernetes.io/name': 'test-app',
'app.kubernetes.io/part-of': 'mock-app',
'app.openshift.io/runtime-version': 'latest',
},
name: 'test-app',
namespace: 'mock-project',
},
spec: {
ports: [
{
name: '8080-tcp',
port: 8080,
protocol: 'TCP',
targetPort: 8080,
},
],
selector: {
app: 'test-app',
deploymentconfig: 'test-app',
},
},
});
});

it('should match the previous snapshot created with deploy image data', () => {
it('should build the expected service object for deploy image data', () => {
const mockDeployImageData: DeployImageFormData = _.cloneDeep(mockDeployImageFormData);
const serviceObj = createService(mockDeployImageData);
expect(serviceObj).toMatchSnapshot();
expect(serviceObj).toStrictEqual({
apiVersion: 'v1',
kind: 'Service',
metadata: {
annotations: {
'openshift.io/generated-by': 'OpenShiftWebConsole',
},
labels: {
app: 'test-app',
'app.kubernetes.io/component': 'test-app',
'app.kubernetes.io/instance': 'test-app',
'app.kubernetes.io/name': 'test-app',
'app.kubernetes.io/part-of': 'mock-app',
'app.openshift.io/runtime-version': 'latest',
},
name: 'test-app',
namespace: 'mock-project',
},
spec: {
ports: [],
selector: {
app: 'test-app',
deploymentconfig: 'test-app',
},
},
});
});

it('should expose only the custom port as TargetPort when it is set', () => {
Expand Down Expand Up @@ -122,18 +181,82 @@ describe('Shared submit utils', () => {
});
});

it('should match the previous snapshot with git import data', () => {
it('should build the expected route object for git import data', () => {
const mockData: GitImportFormData = _.cloneDeep(mockFormData);
mockData.route.targetPort = '8080-tcp';
const routeObj = createRoute(mockData);
expect(routeObj).toMatchSnapshot();
expect(routeObj).toStrictEqual({
apiVersion: 'route.openshift.io/v1',
kind: 'Route',
metadata: {
annotations: {
'app.openshift.io/vcs-ref': '',
'app.openshift.io/vcs-uri': 'https://github.com/test/repo',
'openshift.io/generated-by': 'OpenShiftWebConsole',
},
labels: {
app: 'test-app',
'app.kubernetes.io/component': 'test-app',
'app.kubernetes.io/instance': 'test-app',
'app.kubernetes.io/name': 'test-app',
'app.kubernetes.io/part-of': 'mock-app',
'app.openshift.io/runtime-version': 'latest',
},
name: 'test-app',
namespace: 'mock-project',
},
spec: {
host: '',
path: '',
port: {
targetPort: '8080-tcp',
},
tls: null,
to: {
kind: 'Service',
name: 'test-app',
},
wildcardPolicy: 'None',
},
});
});

it('should match the previous snapshot deploy image data', () => {
it('should build the expected route object for deploy image data', () => {
const mockDeployImageData: DeployImageFormData = _.cloneDeep(mockDeployImageFormData);
mockDeployImageData.route.targetPort = '8080-tcp';
const routeObj = createRoute(mockDeployImageData);
expect(routeObj).toMatchSnapshot();
expect(routeObj).toStrictEqual({
apiVersion: 'route.openshift.io/v1',
kind: 'Route',
metadata: {
annotations: {
'openshift.io/generated-by': 'OpenShiftWebConsole',
},
labels: {
app: 'test-app',
'app.kubernetes.io/component': 'test-app',
'app.kubernetes.io/instance': 'test-app',
'app.kubernetes.io/name': 'test-app',
'app.kubernetes.io/part-of': 'mock-app',
'app.openshift.io/runtime-version': 'latest',
},
name: 'test-app',
namespace: 'mock-project',
},
spec: {
host: '',
path: '',
port: {
targetPort: '8080-tcp',
},
tls: null,
to: {
kind: 'Service',
name: 'test-app',
},
wildcardPolicy: 'None',
},
});
});
});
});
18 changes: 15 additions & 3 deletions frontend/packages/eslint-plugin-console/lib/config/rules/jest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ module.exports = {
'jest/no-identical-title': 'error',
// Disallow Jasmine globals
'jest/no-jasmine-globals': 'error',
// Limited snapshot sizes to keep snapshops manageable and reviewable.
'jest/no-large-snapshots': 'off',

// For better failure messages, use `toHaveLength()` to on object lengths.
'jest/prefer-to-have-length': 'error',
// Suggest using toMatchInlineSnapshot()
// Inline snapshots are disallowed via jest/no-restricted-matchers
'jest/prefer-inline-snapshots': 'off',
// For better failure messages, use `toBeNull()` to assert on null values.
'jest/prefer-to-be-null': 'error',
Expand Down Expand Up @@ -65,4 +64,17 @@ module.exports = {
'jest/no-mocks-import': 'error',
// Disallow commented out tests
'jest/no-commented-out-tests': 'error',
// Prefer explicit assertions over snapshots (aligns with gen-rtl-test / RTL hooks guidance)
'jest/no-restricted-matchers': [
'error',
{
toMatchSnapshot:
'Do not use toMatchSnapshot(); use explicit assertions (e.g. toStrictEqual on focused objects, getByRole, etc.).',
toMatchInlineSnapshot: 'Do not use toMatchInlineSnapshot(); use explicit assertions instead.',
toThrowErrorMatchingSnapshot:
'Do not use toThrowErrorMatchingSnapshot(); assert on error message or type explicitly.',
toThrowErrorMatchingInlineSnapshot:
'Do not use toThrowErrorMatchingInlineSnapshot(); assert on error message or type explicitly.',
},
],
};
13 changes: 12 additions & 1 deletion frontend/packages/eslint-plugin-console/lib/config/testing-library-tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,28 @@
* - `no-wait-for-side-effects` (replaces the removed `no-wait-for-empty-callback` in older plugin majors)
*
* The following **additional** rules are turned on, which are not enabled in `plugin:testing-library/react` by default.
*
* `jest/no-restricted-matchers` is applied here (not via `extends: ['plugin:console/jest']`) so snapshot
* matchers are blocked under the same test `files` globs as RTL rules, without enabling the full Jest
* ruleset from `rules/jest.js` (that preset is still omitted from `react-typescript-prettier`; turning
* it on globally would surface many legacy violations).
*/
const jestRules = require('./rules/jest');

module.exports = {
overrides: [
{
files: ['**/__tests__/**/*.[jt]s?(x)', '**/?(*.)+(spec|test).[jt]s?(x)'],
plugins: ['testing-library'],
plugins: ['testing-library', 'jest'],
env: {
'jest/globals': true,
},
extends: ['plugin:testing-library/react'],
rules: {
'testing-library/prefer-explicit-assert': 'error',
'testing-library/prefer-user-event': 'error',
'testing-library/prefer-user-event-setup': 'error',
'jest/no-restricted-matchers': jestRules['jest/no-restricted-matchers'],
},
},
],
Expand Down
Loading