Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/puny-turtles-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
"@biomejs/biome": patch
---

Fixed [#7205](https://github.com/biomejs/biome/issues/7205): The [`noDuplicateTestHooks`](https://biomejs.dev/linter/rules/no-duplicate-test-hooks/) rule now treats chained describe variants (e.g., describe.each/for/todo) as proper describe scopes, eliminating false positives.

The following code will no longer be a false positive:

```js
describe("foo", () => {
describe.for([])("baz", () => {
beforeEach(() => {});
});

describe.todo("qux", () => {
beforeEach(() => {});
});

describe.todo.each([])("baz", () => {
beforeEach(() => {});
});
});
```
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ test1.js:1:10 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━
```block
test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× The test hook beforeEach is used multiple times in the same test block.
× Duplicate beforeEach hook found.

2 │ describe("foo", () => {
3 │ beforeEach(() => {});
Expand All @@ -80,7 +80,7 @@ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━
5 │ test("bar", () => {
6 │ someFn();

i The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them.
i Remove this duplicate hook or consolidate the logic into a single hook.


```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ test1.js:3:18 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━
```block
test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× The test hook beforeEach is used multiple times in the same test block.
× Duplicate beforeEach hook found.

2 │ describe("foo", () => {
3 │ beforeEach(() => {});
Expand All @@ -86,7 +86,7 @@ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━
5 │ test("bar", () => {
6 │ someFn();

i The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them.
i Remove this duplicate hook or consolidate the logic into a single hook.


```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ test1.js:1:10 lint/suspicious/noFocusedTests FIXABLE ━━━━━━━━
```block
test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× The test hook beforeEach is used multiple times in the same test block.
× Duplicate beforeEach hook found.

2 │ describe("foo", () => {
3 │ beforeEach(() => {});
Expand All @@ -83,7 +83,7 @@ test2.js:4:5 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━
5 │ test("bar", () => {
6 │ someFn();

i The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them.
i Remove this duplicate hook or consolidate the logic into a single hook.


```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ lint ━━━━━━━━━━━━━━━━━━━━━━━━━
```block
test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× The test hook beforeEach is used multiple times in the same test block.
× Duplicate beforeEach hook found.

3 │ beforeEach(() => {
4 │ });
Expand All @@ -56,7 +56,7 @@ test.js:5:2 lint/suspicious/noDuplicateTestHooks ━━━━━━━━━━
7 │ test("bar", () => {
8 │ someFn();

i The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them.
i Remove this duplicate hook or consolidate the logic into a single hook.


```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,24 +121,8 @@ impl Visitor for DuplicateHooksVisitor {
};

// When the visitor enters a function node, push a new entry on the stack
if let Ok(callee) = node.callee() {
if callee.contains_a_test_pattern() == Ok(true) {
if let Some(function_name) = callee.get_callee_object_name()
&& function_name.text_trimmed() == "describe"
{
self.stack.push(HooksContext::default());
}
}
// describe.each has a different syntax
else if let AnyJsExpression::JsCallExpression(call_expression) = callee
&& let Ok(callee) = call_expression.callee()
&& matches!(
callee.to_trimmed_text().text(),
"describe.each" | "describe.only.each" | "fdescribe.each"
)
{
self.stack.push(HooksContext::default());
}
if Self::is_test_describe_call(&node) {
self.stack.push(HooksContext::default());
}

if let Ok(AnyJsExpression::JsIdentifierExpression(identifier)) = node.callee() {
Expand Down Expand Up @@ -166,10 +150,7 @@ impl Visitor for DuplicateHooksVisitor {
// When the visitor exits a function, if it matches the node of the top-most
// entry of the stack and the `has_yield` flag is `false`, emit a query match
if let Some(node) = JsCallExpression::cast_ref(node)
&& let Ok(callee) = node.callee()
&& callee.contains_a_test_pattern() == Ok(true)
&& let Some(function_name) = callee.get_callee_object_name()
&& function_name.text_trimmed() == "describe"
&& Self::is_test_describe_call(&node)
{
self.stack.pop();
}
Expand All @@ -178,6 +159,36 @@ impl Visitor for DuplicateHooksVisitor {
}
}

impl DuplicateHooksVisitor {
/// Determines if a [call expression] is a `describe.` call by checking:
/// 1. The node must be a test call expression
/// e.g. `it.only`, `describe.skip`, `test`
/// 2. The test call must be a `describe.` call
/// first section = `describe` | `fdescribe` | `xdescribe` e.g. `describe.only`, `describe.skip`
/// or second section = `describe` e.g. `test.describe`
///
/// [call expression]: crate::JsCallExpression
fn is_test_describe_call(node: &JsCallExpression) -> bool {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a small doc comment that explains the business logic of the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried, is that good enough?

if let Ok(callee) = node.callee()
&& node.is_test_call_expression() == Ok(true)
{
let text = callee.to_trimmed_text();
let mut split = text.split('.');

let first = split.next();
let second = split.next();

if matches!(first, Some("describe" | "fdescribe" | "xdescribe"))
|| second == Some("describe")
{
return true;
}
}

false
}
}

// Declare a query match struct type containing a JavaScript function node
pub struct DuplicateHooks(JsCallExpression);

Expand Down Expand Up @@ -229,10 +240,11 @@ impl Rule for NoDuplicateTestHooks {
rule_category!(),
ctx.query().range(),
markup! {
"The test hook "<Emphasis>{node_name.text_trimmed()}</Emphasis>" is used multiple times in the same test block."
"Duplicate "<Emphasis>{node_name.text_trimmed()}</Emphasis>" hook found."
},
).note(markup! {
"The presence of the same hook more than once in the same test block can create unexpected errors inside tests. Remove one of them."
)
.note(markup! {
"Remove this duplicate hook or consolidate the logic into a single hook."
}),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Rule for NoSkippedTests {

if node.is_test_call_expression().ok()? {
let callee = node.callee().ok()?;
if callee.contains_a_test_pattern().ok()? {
if callee.contains_a_test_pattern() {
let function_name = callee.get_callee_member_name()?;

if FUNCTION_NAMES.contains(&function_name.text_trimmed()) {
Expand Down
Loading
Loading