Skip to content
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

WP-Scripts: Do not apply jest eslint rules if cypress is installed. #42952

Closed
peterwilsoncc opened this issue Aug 4, 2022 · 2 comments · Fixed by #43272
Closed

WP-Scripts: Do not apply jest eslint rules if cypress is installed. #42952

peterwilsoncc opened this issue Aug 4, 2022 · 2 comments · Fixed by #43272
Assignees
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Package] ESLint plugin /packages/eslint-plugin [Package] Scripts /packages/scripts
Projects

Comments

@peterwilsoncc
Copy link
Contributor

peterwilsoncc commented Aug 4, 2022

What problem does this address?

Using WP-Scripts in combination with Cypress for E2E testing will result in false positives for Jest related rules as the Jest config is unable to pick up Cypress assertions.

What is your proposed solution?

Test for Cypress and disable to Jest rules if the package is installed. This is a similar approach to that taken for Playwrite

if (
isPackageInstalled( 'jest' ) &&
! isPackageInstalled( '@playwright/test' )
) {

Alternative

A configuration option in package.json or eslintrc.json to prevent the Jest config from being included. This will allow the package to be managed generally rather than via snowflake rules for various set ups.

For the time being I have added the following to my config file but it doesn't seem optimal:

{
  "extends": [ "plugin:@wordpress/eslint-plugin/recommended" ],
  "rules": {
		"jest/no-disabled-tests": 0,
		"jest/no-focused-tests": 0,
		"jest/no-identical-title": 0,
		"jest/prefer-to-have-length": 0,
		"jest/valid-expect": 0,
		"jest/expect-expect": 0
  }
}
@peterwilsoncc peterwilsoncc added [Package] Scripts /packages/scripts [Package] ESLint plugin /packages/eslint-plugin labels Aug 4, 2022
@gziolo
Copy link
Member

gziolo commented Aug 12, 2022

At the moment, the Gutenberg project uses both Jest and Playwright for e2e tests. I see that other projects might use other tools, so I believe the best way to move forward would be to make that support fully optional. In effect, we would have to remove those lines:

{
// End-to-end test files and their helpers only.
files: [ '**/specs/**/*.js', '**/?(*.)spec.js' ],
extends: [ require.resolve( './test-e2e.js' ) ],
},

We should also document better existing rulesets in case someone wants to use the one for e2e we are about to disable:

https://github.com/WordPress/gutenberg/tree/trunk/packages/eslint-plugin#rulesets

@gziolo gziolo added Needs Dev Ready for, and needs developer efforts Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Aug 12, 2022
@gziolo gziolo added this to Triage in Core JS via automation Aug 12, 2022
@gziolo gziolo moved this from Triage to To do in Core JS Aug 12, 2022
@gziolo
Copy link
Member

gziolo commented Aug 16, 2022

#43272 should resolve the issue.

@gziolo gziolo moved this from To do to In progress in Core JS Aug 16, 2022
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Aug 16, 2022
Core JS automation moved this from In progress to Done Aug 17, 2022
@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Sep 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Dev Ready for, and needs developer efforts [Package] ESLint plugin /packages/eslint-plugin [Package] Scripts /packages/scripts
Projects
No open projects
Core JS
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants