search

kentcdodds / kcd-scripts

CLI toolbox for common scripts for my projects
http://npm.im/kcd-scripts
MIT License
885 stars 207 forks source link

istanbul ignore else #218

Open ph-fritsche opened 3 years ago

ph-fritsche commented 3 years ago

Relevant code or config

https://github.com/testing-library/user-event/pull/706/files#diff-79de2fcdb58def5d2000c20beddc4bb5a30488589e20c207181967dd0e75aa72R28

What you did:

Run CI with "kcd-scripts": "^11.1.0".

What happened:

After release of v11.2.0 CI fails due to uncovered lines. Turns out istanbul expects the /* istanbul ignore else */ right in between else if now.

The pre-commit hook reverts the change and therefore commits fixing this have to be committed with --no-verify.

Reproduction repository:

https://github.com/testing-library/user-event/pull/706

Problem description:

kcd-scripts lint does not flag /* istanbul ignore else */ else if. kcd-scripts/husky adds a pre-commit hook that changes the (now) correct else /* istanbul ignore else */ if back to /* istanbul ignore else */ else if.

Suggested solution:

Report a lint error for /* istanbul ignore else */ else if. Lint-fix to else /* istanbul ignore else */ if.

kentcdodds commented 3 years ago

I'm not sure I understand what's going on. Is this a linting issue or a prettier issue? Either way I'm not sure what we could do in this project to fix that.

ph-fritsche commented 3 years ago

It's both The linter does not report an error for /* istanbul ignore else */ else if although Istanbul does not honor this annotation. The prettier does change the correct else /* istanbul ignore else */ if to /* istanbul ignore else */ else if.

If we can't fix this in this repo by adjusting some config, then maybe we can relay this problem to the dependency that could fix this. Otherwise it will serve as a reference for the next poor soul that updates to v11.2 and suddenly has a coverage quota violation without touching code or tests. ;)

kentcdodds commented 3 years ago

Yeah, I have no idea what caused the change in behavior and I'm afraid I don't have the bandwidth to investigate it :-/

eps1lon commented 3 years ago

We had the same problem in dom-testing-library. I just converted the else if to an else { if }: https://github.com/testing-library/dom-testing-library/pull/1013