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

fix(js_analyzer): don't flag duplicate hooks in describe.each for lint/suspicious/noDuplicateTestHooks #3806

Merged

Conversation

vohoanglong0107
Copy link
Contributor

Summary

Fixes #3291

Since noDuplicateTestHooks didn't know that describe.each has a different syntax from describe

Test Plan

Added a new test for describe.each hook

…/suspicious/noDuplicateTestHooks

Since this rule didn't recognize describe.each has a different syntax
from describe
@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Sep 6, 2024
Comment on lines -315 to -319
90 │ describe.each(["world"])("%s", () => {
> 91 │ beforeEach(() => {});
│ ^^^^^^^^^^^^^^^^^^^^
92 │ beforeEach(() => {});
93 │
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is undesirable behavior. We should only flag the second beforeEach hook

Comment on lines +315 to +319
76 │ describe.each(["hello"])("%s", () => {
77 │ beforeEach(() => {});
> 78 │ beforeEach(() => {});
│ ^^^^^^^^^^^^^^^^^^^^
79 │
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case was previously missed

Copy link

codspeed-hq bot commented Sep 6, 2024

CodSpeed Performance Report

Merging #3806 will not alter performance

Comparing vohoanglong0107:fix-no-duplicate-test-hooks (c285332) with main (a3446a7)

Summary

✅ 107 untouched benchmarks

@vohoanglong0107 vohoanglong0107 marked this pull request as ready for review September 6, 2024 07:41
@vohoanglong0107 vohoanglong0107 changed the title fix(js_analyzer): don't flag duplicate hooks in describe.each in lint/suspicious/noDuplicateTestHooks fix(js_analyzer): don't flag duplicate hooks in describe.each for lint/suspicious/noDuplicateTestHooks Sep 6, 2024
@vohoanglong0107 vohoanglong0107 requested a review from a team September 10, 2024 02:09
@ematipico ematipico requested a review from a team September 10, 2024 04:40
@vohoanglong0107 vohoanglong0107 merged commit d7070f1 into biomejs:main Sep 10, 2024
13 checks passed
@vohoanglong0107 vohoanglong0107 deleted the fix-no-duplicate-test-hooks branch September 10, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Area: linter L-JavaScript Language: JavaScript and super languages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

💅 lint/suspicious/noDuplicateTestHooks : Duplicate test hook when using describe.each
2 participants