Merge pull request #399 from recca0120/fix/test-duplicates-390

fix: resolve test duplicates and unify dataset logic (#390)

Author: recca0120

packages/extension/src/TestCollection/TestCollection.test.ts

@@ -1763,6 +1763,28 @@ class NoDatasetTest extends TestCase
             }
         });
 
+        it('reset should clear testsuite and namespace nodes from tree (#390)', async () => {
+            const collection = givenTestCollection(`
+                <testsuites>
+                    <testsuite name="Unit">
+                        <directory>tests/Unit</directory>
+                    </testsuite>
+                    <testsuite name="Feature">
+                        <directory>tests/Feature</directory>
+                    </testsuite>
+                </testsuites>`);
+
+            await collection.add(URI.file(phpUnitProject('tests/Unit/ExampleTest.php')));
+            await collection.add(URI.file(phpUnitProject('tests/Feature/ExampleTest.php')));
+
+            expect(ctrl.items.size).toBeGreaterThan(0);
+
+            collection.reset();
+
+            // After reset, the VS Code tree should be completely empty
+            expect(ctrl.items.size).toBe(0);
+        });
+
         it('add test — all fixture files discovered', async () => {
             const collection = givenTestCollection(`
                 <testsuites>

packages/extension/src/TestCollection/TestCollection.ts

@@ -195,13 +195,11 @@ export class TestCollection extends BaseTestCollection {
     private handleReset() {
         const workspaceDefs = this.index.getDefinitionsByType(TestType.workspace);
 
-        for (const [testItem] of this.index.getDefinitionsByType(TestType.class)) {
-            if (!testItem.parent) {
-                this.rootItems.delete(testItem.id);
-                continue;
+        const keepIds = new Set(workspaceDefs.map(([item]) => item.id));
+        for (const [id] of this.rootItems) {
+            if (!keepIds.has(id)) {
+                this.rootItems.delete(id);
             }
-
-            testItem.parent.children.delete(testItem.id);
         }
         this.index.clear();
 

packages/phpunit/docs/data-provider-guide.md

@@ -67,10 +67,10 @@ Teamcity fallback components:
 
 | Component | Description |
 |-----------|------------|
-| `resolveDatasetDefinition` | Parses `#N` and `"name"` formats from Teamcity output |
+| `DatasetExpander.fromTestOutput` | Parses `#N` and `"name"` formats from Teamcity output |
+| `DatasetExpander.fromAnnotations` | Creates dataset children from static analysis labels |
 | `isDatasetResult` | Used by TestResultObserver to distinguish dataset vs non-dataset results |
 | `DatasetObserver` | Dynamically creates dataset child TestItem nodes from Teamcity output after execution |
-| `createDatasetDefinition` | Unified constructor for dataset TestDefinition |
 
 ---
 
@@ -635,10 +635,10 @@ All patterns that AST cannot statically resolve are dynamically populated with d
 
 | Component | Status | Description |
 |-----------|:------:|------------|
-| `resolveDatasetDefinition` | ✅ | Parses `with data set #N` and `with data set "name"` formats from Teamcity output |
+| `DatasetExpander.fromTestOutput` | ✅ | Parses `with data set #N` and `with data set "name"` formats from Teamcity output |
+| `DatasetExpander.fromAnnotations` | ✅ | Creates dataset children from static analysis labels, ensures consistent format |
 | `isDatasetResult` | ✅ | Used by TestResultObserver to distinguish dataset vs non-dataset results |
 | `DatasetObserver` | ✅ | Dynamically creates dataset child TestItem nodes from Teamcity output after execution |
-| `createDatasetDefinition` | ✅ | Unified constructor for dataset TestDefinition, ensures consistent format |
 
 ### Fallback Flow
 

packages/phpunit/docs/data-provider-guide.zh-TW.md

@@ -67,10 +67,10 @@ Teamcity 補漏元件：
 
 | 元件 | 說明 |
 |------|------|
-| `resolveDatasetDefinition` | 解析 Teamcity 輸出中的 `#N` 和 `"name"` 格式 |
+| `DatasetExpander.fromTestOutput` | 解析 Teamcity 輸出中的 `#N` 和 `"name"` 格式 |
+| `DatasetExpander.fromAnnotations` | 從靜態分析 labels 建立 dataset children |
 | `isDatasetResult` | TestResultObserver 用來分流 dataset vs 非 dataset 結果 |
 | `DatasetObserver` | 執行後從 Teamcity 輸出動態建立 dataset 子 TestItem |
-| `createDatasetDefinition` | 統一建構 dataset TestDefinition |
 
 ---
 
@@ -635,10 +635,10 @@ testStarted name='it business closed with data set "(|'School|', |'Sunday|')"'
 
 | 元件 | 狀態 | 說明 |
 |------|:----:|------|
-| `resolveDatasetDefinition` | ✅ | 解析 Teamcity 輸出中的 `with data set #N` 和 `with data set "name"` 格式 |
+| `DatasetExpander.fromTestOutput` | ✅ | 解析 Teamcity 輸出中的 `with data set #N` 和 `with data set "name"` 格式 |
+| `DatasetExpander.fromAnnotations` | ✅ | 從靜態分析 labels 建立 dataset children，確保格式一致 |
 | `isDatasetResult` | ✅ | TestResultObserver 用來分流 dataset vs 非 dataset 結果 |
 | `DatasetObserver` | ✅ | 執行後從 Teamcity 輸出動態建立 dataset 子 TestItem 節點 |
-| `createDatasetDefinition` | ✅ | 統一建構 dataset TestDefinition，確保格式一致 |
 
 ### 補漏流程
 

packages/phpunit/src/Interpreter/Expressions/Functions/Array/ArrayCombineExpression.ts

@@ -1,4 +1,4 @@
-import { datasetIndexed, datasetNamed } from '../../../../utils';
+import { datasetExpander } from '../../../../TestParser/DatasetExpander';
 import type { AstNode, CallNode } from '../../../AstParser/AstNode';
 import type { Context, Expression } from '../../Expression';
 
@@ -19,7 +19,7 @@ class ArrayCombineExpression implements Expression<string[]> {
             return undefined;
         }
         return [...resolved.values()].map((item, i) =>
-            typeof item === 'string' ? datasetNamed(item) : datasetIndexed(i),
+            typeof item === 'string' ? datasetExpander.named(item) : datasetExpander.indexed(i),
         );
     }
 }

packages/phpunit/src/Interpreter/Expressions/PhpExpression.ts

@@ -1,4 +1,4 @@
-import { datasetIndexed, datasetNamed } from '../../utils';
+import { datasetExpander } from '../../TestParser/DatasetExpander';
 import type { AstNode } from '../AstParser/AstNode';
 import type { Bindings, Context, Expression } from './Expression';
 import { arrayCombineExpression } from './Functions/Array/ArrayCombineExpression';
@@ -106,7 +106,7 @@ export function extractLabels(resolved: unknown): string[] | undefined {
         return undefined;
     }
     return [...resolved.keys()].map((key: string) =>
-        /^\d+$/.test(key) ? datasetIndexed(key) : datasetNamed(key),
+        /^\d+$/.test(key) ? datasetExpander.indexed(key) : datasetExpander.named(key),
     );
 }
 

packages/phpunit/src/Interpreter/Expressions/Statements/LoopBodyExecutor.ts

@@ -1,4 +1,4 @@
-import { datasetIndexed, datasetNamed } from '../../../utils';
+import { datasetExpander } from '../../../TestParser/DatasetExpander';
 import type { AstNode, IfStatementNode } from '../../AstParser/AstNode';
 import type { Context } from '../Expression';
 export const MAX_ITERATIONS = 1000;
@@ -47,9 +47,9 @@ const bodyStatementHandlers: Record<string, (stmt: AstNode, ctx: LoopContext) =>
     yield_expression: (stmt, ctx) => {
         const resolvedKey = ctx.context.resolve(stmt);
         if (resolvedKey !== undefined) {
-            ctx.labels.push(datasetNamed(String(resolvedKey)));
+            ctx.labels.push(datasetExpander.named(String(resolvedKey)));
         } else {
-            ctx.labels.push(datasetIndexed(ctx.numericIndex++));
+            ctx.labels.push(datasetExpander.indexed(ctx.numericIndex++));
         }
         return undefined;
     },

packages/phpunit/src/Interpreter/Resolvers/PestResolver.ts

@@ -1,4 +1,4 @@
-import { datasetNamed } from '../../utils';
+import { datasetExpander } from '../../TestParser/DatasetExpander';
 import type { AstNode, CallNode, ExpressionStatementNode } from '../AstParser/AstNode';
 import { evaluate } from '../Expressions/PhpExpression';
 import type { PHP } from '../PHP';
@@ -134,6 +134,7 @@ function resolveDatasets(sources: AstNode[]): string[] {
         return cartesianProduct(datasets);
     }
 
+    // Cartesian failed (e.g. closure/generator can't be statically resolved); fall back to independent resolution
     return sources.flatMap((source) => resolvePestLabels(source));
 }
 
@@ -154,18 +155,43 @@ function resolvePestLabels(node: AstNode): string[] {
             if (!formatted) {
                 return [];
             }
-            labels.push(datasetNamed(formatted));
+            labels.push(datasetExpander.named(formatted));
         } else {
-            labels.push(datasetNamed(`dataset "${key}"`));
+            labels.push(datasetExpander.named(`dataset "${key}"`));
         }
     }
-    return labels;
+    return deduplicateLabels(labels);
 }
 
+function deduplicateLabels(labels: string[]): string[] {
+    const counts = new Map<string, number>();
+    for (const label of labels) {
+        counts.set(label, (counts.get(label) ?? 0) + 1);
+    }
+
+    const indices = new Map<string, number>();
+    return labels.map((label) => {
+        if ((counts.get(label) ?? 0) <= 1) {
+            return label;
+        }
+        const idx = (indices.get(label) ?? 0) + 1;
+        indices.set(label, idx);
+        return `${label} #${idx}`;
+    });
+}
+
+const MAX_DATASET_ITEMS = 3;
+
 function formatPestValue(value: unknown): string | undefined {
     if (value instanceof Map || Array.isArray(value)) {
         const items = value instanceof Map ? [...value.values()] : value;
-        return `(${items.map((v) => (typeof v === 'string' ? `'${v}'` : String(v))).join(', ')})`;
+        const truncated = items.length > MAX_DATASET_ITEMS;
+        const visible = items.slice(0, MAX_DATASET_ITEMS);
+        const parts = visible.map((v) => (typeof v === 'string' ? `'${v}'` : String(v)));
+        if (truncated) {
+            parts.push('…');
+        }
+        return `(${parts.join(', ')})`;
     }
     if (typeof value === 'string') {
         return `('${value}')`;
@@ -178,7 +204,9 @@ function cartesianProduct(datasets: string[][]): string[] {
     for (let i = 1; i < datasets.length; i++) {
         combinations = combinations.flatMap((combo) => datasets[i].map((v) => [...combo, v]));
     }
-    return combinations.map((combo) => datasetNamed(combo.map((v) => `('${v}')`).join(' / ')));
+    return combinations.map((combo) =>
+        datasetExpander.named(combo.map((v) => `('${v}')`).join(' / ')),
+    );
 }
 
 function unwrapClosureBody(node: AstNode): AstNode | undefined {

packages/phpunit/src/Interpreter/Resolvers/TestTagResolver.ts

@@ -1,5 +1,5 @@
+import { datasetExpander } from '../../TestParser/DatasetExpander';
 import type { Annotations } from '../../types';
-import { datasetIndexed, datasetNamed } from '../../utils';
 import type { AstNode, MethodNode } from '../AstParser/AstNode';
 import type { PHP } from '../PHP';
 import type { Annotatable, Resolver } from '../types';
@@ -98,12 +98,12 @@ export class TestTagResolver implements Resolver {
                 const name = attr.args[1];
                 dataset.push(
                     typeof name === 'string' && name
-                        ? datasetNamed(name)
-                        : datasetIndexed(datasetIndex),
+                        ? datasetExpander.named(name)
+                        : datasetExpander.indexed(datasetIndex),
                 );
                 datasetIndex++;
             } else if (attr.name === 'TestWithJson') {
-                dataset.push(datasetIndexed(datasetIndex));
+                dataset.push(datasetExpander.indexed(datasetIndex));
                 datasetIndex++;
             }
         }

packages/phpunit/src/ProcessBuilder/FilterStrategy.ts

@@ -1,6 +1,6 @@
 import { TestIdentifier } from '../TestIdentifier/TestIdentifier';
+import { datasetExpander } from '../TestParser/DatasetExpander';
 import { type TestDefinition, TestType } from '../types';
-import { parseDataset } from '../utils';
 
 abstract class FilterStrategy {
     constructor(protected testDefinition: TestDefinition) {}
@@ -93,7 +93,7 @@ class DatasetFilterStrategy extends DescribeFilterStrategy {
 
     protected getMethodNamePattern() {
         const methodName = (this.testDefinition.methodName ?? '').trim();
-        const { dataset } = parseDataset(this.testDefinition.id ?? '');
+        const { dataset } = datasetExpander.parse(this.testDefinition.id ?? '');
 
         const escapedMethod = TestIdentifier.generateSearchText(methodName).replace(/'/g, "\\'");
         return `${escapedMethod}${dataset.replace(/[.*+?^${}()|[\]\\/']/g, '\\$&')}`;