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

feat(biome_js_analyze): update the methods of noFocusedTests #1999

Merged
merged 4 commits into from
Mar 11, 2024

Conversation

chansuke
Copy link
Member

@chansuke chansuke commented Mar 8, 2024

Summary

Fixes #597

Test Plan

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 8, 2024
Copy link

netlify bot commented Mar 8, 2024

Deploy Preview for biomejs canceled.

Name Link
🔨 Latest commit 76394f7
🔍 Latest deploy log https://app.netlify.com/sites/biomejs/deploys/65ee9cbc04abf00008827214

@github-actions github-actions bot added the A-Website Area: website label Mar 8, 2024
@Conaclos
Copy link
Member

Conaclos commented Mar 8, 2024

I misunderstood your question. Why should we rule out describe.only and the like?

Copy link

codspeed-hq bot commented Mar 8, 2024

CodSpeed Performance Report

Merging #1999 will not alter performance

Comparing chansuke:feat/update-no-focused-tests (76394f7) with main (75eed2e)

Summary

✅ 93 untouched benchmarks

@chansuke
Copy link
Member Author

chansuke commented Mar 8, 2024

@Conaclos
Apologies for the confusion 🙇
Based on your statement

I think we could restrict the rule to test.only, describe.only and it.only'

I understood that we needed to limit the rule specifically to the describe, it, and test methods...

https://github.com/biomejs/biome/blob/0986acd6f859a08dda59b5990e267a8643be85a9/crates/biome_js_analyze/tests/specs/nursery/noFocusedTests/invalid.js

@Conaclos
Copy link
Member

Conaclos commented Mar 8, 2024

@Conaclos Apologies for the confusion 🙇 Based on your statement

I think we could restrict the rule to test.only, describe.only and it.only'

I understood that we needed to limit the rule specifically to the describe, it, and test methods...

https://github.com/biomejs/biome/blob/0986acd6f859a08dda59b5990e267a8643be85a9/crates/biome_js_analyze/tests/specs/nursery/noFocusedTests/invalid.js

You are right.
However, support for fdescribe and fit was added later in #1858 and #597 was not updated...
Unfortunately it seems that #1641, #1858, and #1864 were unaware of #597.

Could you add again the support of fdescribe and fit?
Otherwise, the changes you brought looks good to me :)
Sorry for the confusion, I was as confused as you are ^^

@chansuke chansuke changed the title feat(biome_js_analyze): restrict the methods of noFocusedTests feat(biome_js_analyze): update the methods of noFocusedTests Mar 9, 2024
@chansuke chansuke force-pushed the feat/update-no-focused-tests branch from 6d6e155 to 6abe775 Compare March 10, 2024 07:02
@Conaclos Conaclos merged commit ffc349c into biomejs:main Mar 11, 2024
19 checks passed
@Conaclos
Copy link
Member

Great job :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Area: linter A-Website Area: website L-JavaScript Language: JavaScript and super languages
Projects
None yet
2 participants