avajs / eslint-plugin-ava

ESLint rules for AVA
https://avajs.dev
MIT License
229 stars 49 forks source link

Add an option to avoid fixing no-skip-test #324

Closed edbrannin closed 3 years ago

edbrannin commented 3 years ago

My team's projects run eslint --fix by deafult, and some of our editors apply lint-fixes on save by default.

This has led to some issues with this rule (with I 100% wish to keep using):

This option also lets projects opt-in to working like the eslint docs recommend:

Best practices for fixes:

  1. Avoid any fixes that could change the runtime behavior of code and cause it to stop working.

If this is accepted, I'd like to do the same thing for no-only-test for the same reasons.

edbrannin commented 3 years ago

Integration test failure:

[17:34:00] Integration tests [failed]

 got (eslint --format json --config /home/runner/work/eslint-plugin-ava/eslint-plugin-ava/test/integration/eslint-config-ava-tester/index.js /tmp/1919053f89b82a9ed6d360e4babe2161 --parser @typescript-eslint/parser --ext .ts)
Command failed with exit code 1: npx eslint --format json --config /home/runner/work/eslint-plugin-ava/eslint-plugin-ava/test/integration/eslint-config-ava-tester/index.js /tmp/1919053f89b82a9ed6d360e4babe2161 --parser @typescript-eslint/parser --ext .ts
Cannot destructure property 'type' of 'node' as it is undefined.
Occurred while linting /tmp/1919053f89b82a9ed6d360e4babe2161/test/cache.ts:363
Cannot destructure property 'type' of 'node' as it is undefined.
Occurred while linting /tmp/1919053f89b82a9ed6d360e4babe2161/test/cache.ts:363

 got (eslint --format json --config /home/runner/work/eslint-plugin-ava/eslint-plugin-ava/test/integration/eslint-config-ava-tester/index.js /tmp/1919053f89b82a9ed6d360e4babe2161 --parser @typescript-eslint/parser --ext .ts --fix-dry-run)
Command failed with exit code 1: npx eslint --format json --config /home/runner/work/eslint-plugin-ava/eslint-plugin-ava/test/integration/eslint-config-ava-tester/index.js /tmp/1919053f89b82a9ed6d360e4babe2161 --parser @typescript-eslint/parser --ext .ts --fix-dry-run
Cannot destructure property 'type' of 'node' as it is undefined.
Occurred while linting /tmp/1919053f89b82a9ed6d360e4babe2161/test/cache.ts:363
Cannot destructure property 'type' of 'node' as it is undefined.
Occurred while linting /tmp/1919053f89b82a9ed6d360e4babe2161/test/cache.ts:363

I'm at a bit of a loss here. The only type I see in my PR is in the schema, but I based that on no-ignored-test-files so I doubt that's where the problem is.

Maybe I'll try using npm link to see if it works in my projects...

edbrannin commented 3 years ago

I tried this on my code with npm link

If you're ok with this change in general, I'd appreciate some help figuring out what went wrong in the integration tests.

sindresorhus commented 3 years ago

See: https://github.com/avajs/eslint-plugin-ava/issues/281#issuecomment-587137623 (Sorry, I should have moved that comment to a separate clear issue)

edbrannin commented 3 years ago

@sindresorhus No worries, that's what I get for raising a PR instead of an issue.

I'm a bit concerned about unit tests breakage, but at least the implementation should be a quick fix from what's there now. I may give that a shot tomorrow.