Skip to content

Commit 7b230ea

Browse files
divybotlittledivy
andauthored
fix(lsp): discover all tests when names are duplicated (#34624)
## Summary The LSP test collector hashes each test's id from `name + parent_chain + specifier`. When a file contains two tests with the same name (e.g. two `Deno.test(function foo() {})` calls), they hashed to the same id and the second one was silently dropped, leaving it invisible in the test explorer. Example from the original report: ```js Deno.test(function addTest() { assertEquals(add(2, 3), 5); }); Deno.test(function foo() {}); Deno.test(function foo() {}); // <- previously not discovered ``` ## Fix Track a per-(parent_id, name) occurrence index (`name_index`) on each `TestDefinition` and fold it into the id hash when non-zero. The static collector counts existing matches in `defs` at registration time; the dynamic test reporter keeps a per-run counter so runtime registrations resolve to the matching static definitions in source order. `name_index == 0` preserves the original hash, so existing test ids — and the assertions in the existing collector unit tests — are unchanged for the common (unique-name) case. Steps with duplicate names inside duplicate-named parents are also handled: a parent's `name_index` is folded into its children's id hash via the parent-chain walk, so two `step` children under two different `foo` parents now get distinct ids. ## Test plan - [x] Existing `cli/lsp/testing/collectors.rs` unit tests pass unchanged. - [x] New regression tests added: - `test_test_collector_duplicate_test_names` — three duplicate-named top-level tests each get a unique id and `name_index` 0/1/2. - `test_test_collector_duplicate_test_names_with_steps` — two `Deno.test("foo", …)` blocks with a `"step"` each both register, and the two steps belong to different parents. Closes #20371 Closes denoland/divybot#355 Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
1 parent 95209b3 commit 7b230ea

3 files changed

Lines changed: 228 additions & 28 deletions

File tree

cli/lsp/testing/collectors.rs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,7 @@ pub mod tests {
701701
"4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
702702
.to_string(),
703703
name: "test".to_string(),
704+
name_index: 0,
704705
range: Some(new_range(1, 11, 1, 15)),
705706
is_dynamic: false,
706707
parent_id: None,
@@ -733,6 +734,7 @@ pub mod tests {
733734
"4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
734735
.to_string(),
735736
name: "test".to_string(),
737+
name_index: 0,
736738
range: Some(new_range(1, 11, 1, 15)),
737739
is_dynamic: false,
738740
parent_id: None,
@@ -773,6 +775,7 @@ pub mod tests {
773775
TestDefinition {
774776
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string(),
775777
name: "test".to_string(),
778+
name_index: 0,
776779
range: Some(new_range(1, 11, 1, 15)),
777780
is_dynamic: false,
778781
parent_id: None,
@@ -784,6 +787,7 @@ pub mod tests {
784787
TestDefinition {
785788
id: "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string(),
786789
name: "step".to_string(),
790+
name_index: 0,
787791
range: Some(new_range(4, 18, 4, 22)),
788792
is_dynamic: false,
789793
parent_id: Some("4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string()),
@@ -795,6 +799,7 @@ pub mod tests {
795799
TestDefinition {
796800
id: "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string(),
797801
name: "sub step".to_string(),
802+
name_index: 0,
798803
range: Some(new_range(5, 18, 5, 22)),
799804
is_dynamic: false,
800805
parent_id: Some("704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string()),
@@ -834,6 +839,7 @@ pub mod tests {
834839
TestDefinition {
835840
id: "4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string(),
836841
name: "test".to_string(),
842+
name_index: 0,
837843
range: Some(new_range(1, 11, 1, 15)),
838844
is_dynamic: false,
839845
parent_id: None,
@@ -845,6 +851,7 @@ pub mod tests {
845851
TestDefinition {
846852
id: "704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string(),
847853
name: "step".to_string(),
854+
name_index: 0,
848855
range: Some(new_range(4, 18, 4, 22)),
849856
is_dynamic: false,
850857
parent_id: Some("4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9".to_string()),
@@ -856,6 +863,7 @@ pub mod tests {
856863
TestDefinition {
857864
id: "0d006a4ec0abaa9cc1d18256b1ccd2677a4c882ff5cb807123890f7528ab1e8d".to_string(),
858865
name: "sub step".to_string(),
866+
name_index: 0,
859867
range: Some(new_range(5, 18, 5, 22)),
860868
is_dynamic: false,
861869
parent_id: Some("704d24083fd4a3e1bd204faa20827dc594334812245e5d45dda222b3edc60a0c".to_string()),
@@ -888,6 +896,7 @@ pub mod tests {
888896
"4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
889897
.to_string(),
890898
name: "test".to_string(),
899+
name_index: 0,
891900
range: Some(new_range(2, 6, 2, 10)),
892901
is_dynamic: false,
893902
parent_id: None,
@@ -920,6 +929,7 @@ pub mod tests {
920929
TestDefinition {
921930
id: "86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2".to_string(),
922931
name: "useFnName".to_string(),
932+
name_index: 0,
923933
range: Some(new_range(1, 11, 1, 15)),
924934
is_dynamic: false,
925935
parent_id: None,
@@ -931,6 +941,7 @@ pub mod tests {
931941
TestDefinition {
932942
id: "dac8a169b8f8c6babf11122557ea545de2733bfafed594d044b22bc6863a0856".to_string(),
933943
name: "step".to_string(),
944+
name_index: 0,
934945
range: Some(new_range(2, 14, 2, 15)),
935946
is_dynamic: false,
936947
parent_id: Some("86b4c821900e38fc89f24bceb0e45193608ab3f9d2a6019c7b6a5aceff5d7df2".to_string()),
@@ -963,6 +974,7 @@ pub mod tests {
963974
"4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
964975
.to_string(),
965976
name: "test".to_string(),
977+
name_index: 0,
966978
range: Some(new_range(2, 6, 2, 7)),
967979
is_dynamic: false,
968980
parent_id: None,
@@ -996,6 +1008,7 @@ pub mod tests {
9961008
"4ebb361c93f76a0f1bac300638675609f1cf481e6f3b9006c3c98604b3a184e9"
9971009
.to_string(),
9981010
name: "test".to_string(),
1011+
name_index: 0,
9991012
range: Some(new_range(2, 11, 2, 15)),
10001013
is_dynamic: false,
10011014
parent_id: None,
@@ -1028,6 +1041,7 @@ pub mod tests {
10281041
TestDefinition {
10291042
id: "87f28e06f5ddadd90a74a93b84df2e31b9edced8301b0ad4c8fbab8d806ec99d".to_string(),
10301043
name: "foo".to_string(),
1044+
name_index: 0,
10311045
range: Some(new_range(2, 16, 2, 22)),
10321046
is_dynamic: false,
10331047
parent_id: None,
@@ -1039,6 +1053,7 @@ pub mod tests {
10391053
TestDefinition {
10401054
id: "e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568".to_string(),
10411055
name: "someFunction".to_string(),
1056+
name_index: 0,
10421057
range: Some(new_range(1, 11, 1, 15)),
10431058
is_dynamic: false,
10441059
parent_id: None,
@@ -1050,6 +1065,7 @@ pub mod tests {
10501065
TestDefinition {
10511066
id: "e1bd61cdaf5e64863d3d85baffe3e43bd57cdb8dc0b5d6a9e03ade18b7f68d47".to_string(),
10521067
name: "bar".to_string(),
1068+
name_index: 0,
10531069
range: Some(new_range(3, 16, 3, 20)),
10541070
is_dynamic: false,
10551071
parent_id: None,
@@ -1084,6 +1100,7 @@ pub mod tests {
10841100
"e0f6a73647b763f82176c98a019e54200b799a32007f9859fb782aaa9e308568"
10851101
.to_string(),
10861102
name: "someFunction".to_string(),
1103+
name_index: 0,
10871104
range: Some(new_range(2, 11, 2, 15)),
10881105
is_dynamic: false,
10891106
parent_id: None,
@@ -1117,6 +1134,7 @@ pub mod tests {
11171134
"6d05d6dc35548b86a1e70acaf24a5bc2dd35db686b35b685ad5931d201b4a918"
11181135
.to_string(),
11191136
name: "Test 3:7".to_string(),
1137+
name_index: 0,
11201138
range: Some(new_range(2, 11, 2, 15)),
11211139
is_dynamic: false,
11221140
parent_id: None,
@@ -1155,6 +1173,7 @@ pub mod tests {
11551173
TestDefinition {
11561174
id: "3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string(),
11571175
name: "1".to_string(),
1176+
name_index: 0,
11581177
range: Some(new_range(1, 11, 1, 15)),
11591178
is_dynamic: false,
11601179
parent_id: None,
@@ -1169,6 +1188,7 @@ pub mod tests {
11691188
TestDefinition {
11701189
id: "e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string(),
11711190
name: "step 1".to_string(),
1191+
name_index: 0,
11721192
range: Some(new_range(2, 16, 2, 20)),
11731193
is_dynamic: false,
11741194
parent_id: Some("3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string()),
@@ -1180,6 +1200,7 @@ pub mod tests {
11801200
TestDefinition {
11811201
id: "d874949e18dfc297e15c52ff13f13b4e6ae911ec1818b2c761e3313bc018a3ab".to_string(),
11821202
name: "nested step".to_string(),
1203+
name_index: 0,
11831204
range: Some(new_range(3, 18, 3, 22)),
11841205
is_dynamic: false,
11851206
parent_id: Some("e714fc695c0895327bf7148a934c3303ad515af029a14906be46f80340c6d7e3".to_string()),
@@ -1191,6 +1212,7 @@ pub mod tests {
11911212
TestDefinition {
11921213
id: "ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string(),
11931214
name: "step 2".to_string(),
1215+
name_index: 0,
11941216
range: Some(new_range(5, 16, 5, 20)),
11951217
is_dynamic: false,
11961218
parent_id: Some("3799fc549a32532145ffc8532b0cd943e025bbc19a02e2cde9be94f87bceb829".to_string()),
@@ -1202,6 +1224,7 @@ pub mod tests {
12021224
TestDefinition {
12031225
id: "96729f1f1608e50160b0bf11946719384b4021fd1d26b14eff7765034b3d2684".to_string(),
12041226
name: "nested step".to_string(),
1227+
name_index: 0,
12051228
range: Some(new_range(6, 18, 6, 22)),
12061229
is_dynamic: false,
12071230
parent_id: Some("ec6b03d3dd3dde78d2d11ed981d3386083aeca701510cc049189d74bd79f8587".to_string()),
@@ -1238,6 +1261,7 @@ pub mod tests {
12381261
TestDefinition {
12391262
id: "87f28e06f5ddadd90a74a93b84df2e31b9edced8301b0ad4c8fbab8d806ec99d".to_string(),
12401263
name: "foo".to_string(),
1264+
name_index: 0,
12411265
range: Some(new_range(1, 6, 1, 14)),
12421266
is_dynamic: false,
12431267
parent_id: None,
@@ -1254,6 +1278,7 @@ pub mod tests {
12541278
TestDefinition {
12551279
id: "821a21d7a8c0a0c09538ad10e683a8ea5907d14048d0ee264bcc300efb86f09d".to_string(),
12561280
name: "foo step 1".to_string(),
1281+
name_index: 0,
12571282
range: Some(new_range(2, 8, 2, 10)),
12581283
is_dynamic: false,
12591284
parent_id: Some("87f28e06f5ddadd90a74a93b84df2e31b9edced8301b0ad4c8fbab8d806ec99d".to_string()),
@@ -1265,6 +1290,7 @@ pub mod tests {
12651290
TestDefinition {
12661291
id: "778232aa6936db81c7f94c67c7ec9c60340f5aea76247fa183798264b690ef56".to_string(),
12671292
name: "foo step 2".to_string(),
1293+
name_index: 0,
12681294
range: Some(new_range(3, 11, 3, 17)),
12691295
is_dynamic: false,
12701296
parent_id: Some("87f28e06f5ddadd90a74a93b84df2e31b9edced8301b0ad4c8fbab8d806ec99d".to_string()),
@@ -1276,6 +1302,7 @@ pub mod tests {
12761302
TestDefinition {
12771303
id: "5be2a4c1f46c2efb816381a3b1a22fe271f59a91e695ad6324780168d9ed17b1".to_string(),
12781304
name: "foo step 3".to_string(),
1305+
name_index: 0,
12791306
range: Some(new_range(4, 11, 4, 15)),
12801307
is_dynamic: false,
12811308
parent_id: Some("87f28e06f5ddadd90a74a93b84df2e31b9edced8301b0ad4c8fbab8d806ec99d".to_string()),
@@ -1287,6 +1314,7 @@ pub mod tests {
12871314
TestDefinition {
12881315
id: "757cd9c4ee3042df742884fd8ebc3f8f60a523c13f09f2b425112424d704d1c1".to_string(),
12891316
name: "foo step 4".to_string(),
1317+
name_index: 0,
12901318
range: Some(new_range(5, 11, 5, 15)),
12911319
is_dynamic: false,
12921320
parent_id: Some("87f28e06f5ddadd90a74a93b84df2e31b9edced8301b0ad4c8fbab8d806ec99d".to_string()),
@@ -1298,6 +1326,7 @@ pub mod tests {
12981326
TestDefinition {
12991327
id: "4e3802608de755b3b67088cc5146f14f5a5757f050f7f506b9c33e94eb4b7c73".to_string(),
13001328
name: "qux".to_string(),
1329+
name_index: 0,
13011330
range: Some(new_range(9, 15, 9, 19)),
13021331
is_dynamic: false,
13031332
parent_id: None,
@@ -1309,6 +1338,7 @@ pub mod tests {
13091338
TestDefinition {
13101339
id: "e1bd61cdaf5e64863d3d85baffe3e43bd57cdb8dc0b5d6a9e03ade18b7f68d47".to_string(),
13111340
name: "bar".to_string(),
1341+
name_index: 0,
13121342
range: Some(new_range(7, 15, 7, 21)),
13131343
is_dynamic: false,
13141344
parent_id: None,
@@ -1320,6 +1350,7 @@ pub mod tests {
13201350
TestDefinition {
13211351
id: "3899772fca803def19cb92f852057a0117e3dfdd0dc6cd25e9180bef221605d4".to_string(),
13221352
name: "baz".to_string(),
1353+
name_index: 0,
13231354
range: Some(new_range(8, 15, 8, 19)),
13241355
is_dynamic: false,
13251356
parent_id: None,
@@ -1332,4 +1363,77 @@ pub mod tests {
13321363
}
13331364
);
13341365
}
1366+
1367+
// Regression test for https://github.com/denoland/deno/issues/20371.
1368+
// Tests with duplicate names at the same level should each be discovered
1369+
// and assigned a unique id (disambiguated via `name_index`).
1370+
#[test]
1371+
fn test_test_collector_duplicate_test_names() {
1372+
let test_module = collect(
1373+
r#"
1374+
Deno.test(function foo() {});
1375+
Deno.test(function foo() {});
1376+
Deno.test(function foo() {});
1377+
"#,
1378+
);
1379+
1380+
let foos: Vec<_> = test_module
1381+
.defs
1382+
.values()
1383+
.filter(|d| d.parent_id.is_none() && d.name == "foo")
1384+
.collect();
1385+
assert_eq!(
1386+
foos.len(),
1387+
3,
1388+
"all three duplicate-named tests should be registered"
1389+
);
1390+
let mut indices: Vec<u32> = foos.iter().map(|d| d.name_index).collect();
1391+
indices.sort();
1392+
assert_eq!(indices, vec![0, 1, 2]);
1393+
let mut ids: Vec<&str> = foos.iter().map(|d| d.id.as_str()).collect();
1394+
ids.sort();
1395+
ids.dedup();
1396+
assert_eq!(
1397+
ids.len(),
1398+
3,
1399+
"each duplicate-named test should have a unique id"
1400+
);
1401+
}
1402+
1403+
// Steps with duplicate names inside duplicate-named parent tests should
1404+
// also each get a unique id.
1405+
#[test]
1406+
fn test_test_collector_duplicate_test_names_with_steps() {
1407+
let test_module = collect(
1408+
r#"
1409+
Deno.test("foo", async (t) => {
1410+
await t.step("step", () => {});
1411+
});
1412+
Deno.test("foo", async (t) => {
1413+
await t.step("step", () => {});
1414+
});
1415+
"#,
1416+
);
1417+
1418+
let foos: Vec<_> = test_module
1419+
.defs
1420+
.values()
1421+
.filter(|d| d.parent_id.is_none() && d.name == "foo")
1422+
.collect();
1423+
assert_eq!(foos.len(), 2);
1424+
let steps: Vec<_> = test_module
1425+
.defs
1426+
.values()
1427+
.filter(|d| d.name == "step")
1428+
.collect();
1429+
assert_eq!(steps.len(), 2);
1430+
// Each foo should own exactly one step.
1431+
for foo in &foos {
1432+
assert_eq!(foo.step_ids.len(), 1);
1433+
}
1434+
// The two steps belong to different parents.
1435+
let parent_ids: std::collections::HashSet<_> =
1436+
steps.iter().map(|d| d.parent_id.clone()).collect();
1437+
assert_eq!(parent_ids.len(), 2);
1438+
}
13351439
}

0 commit comments

Comments
 (0)