Update: no-identical-tests despite of properties order. #21
Conversation
|
@not-an-aardvark, SOS!! all the tests passed, but linting errors. is this the |
|
It seems like the issue is that the rule is reporting a lot of false positives on the codebase. I don't think this is related to |
|
but when adding test cases that reporting errors, it works well. |
|
I think the issue is that it's not accounting for test cases which are just shorthand strings. |
aladdin-add
force-pushed
the
issue17
branch
2 times, most recently
from
a08db57 to
185dd40
Compare
lib/rules/no-identical-tests.js
Outdated
| function eq (ta, tb) { | ||
| if (ta.type !== tb.type) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
An additional enhancement would be to consider these test cases equal:
'foo'{ code: 'foo' }
But that doesn't have to be part of this PR.
lib/rules/no-identical-tests.js
Outdated
| *compare two test cases despite of properties order. | ||
| *@returns {boolean} if eq, return true, else return false. | ||
| */ | ||
| function eq (ta, tb) { |
There was a problem hiding this comment.
Can you give these variables longer names rather than abbreviating? I think the abbreviations make it harder to read the code.
For example: ta => testA, pa => propertiesA, paObj => propertiesObjectA, etc.
lib/rules/no-identical-tests.js
Outdated
| } | ||
|
|
||
| // convert array to object | ||
| const paObj = Object.create(null); |
There was a problem hiding this comment.
Nitpick: It would be better to use a Set rather than an object here.
fixes #17