Skip to content

Commit 2b8c04b

Browse files
committed
refactor: unify dataset logic into DatasetExpander singleton
Consolidate scattered dataset utilities (datasetNamed, datasetIndexed, parseDataset, normalizePestLabel, createDatasetDefinition, resolveDatasetDefinition) into a single DatasetExpander instance. Both static analysis and Teamcity runtime paths now share the same code.
1 parent a0f963b commit 2b8c04b

18 files changed

Lines changed: 140 additions & 155 deletions

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ Teamcity fallback components:
6767

6868
| Component | Description |
6969
|-----------|------------|
70-
| `resolveDatasetDefinition` | Parses `#N` and `"name"` formats from Teamcity output |
70+
| `DatasetExpander.fromTestOutput` | Parses `#N` and `"name"` formats from Teamcity output |
71+
| `DatasetExpander.fromAnnotations` | Creates dataset children from static analysis labels |
7172
| `isDatasetResult` | Used by TestResultObserver to distinguish dataset vs non-dataset results |
7273
| `DatasetObserver` | Dynamically creates dataset child TestItem nodes from Teamcity output after execution |
73-
| `createDatasetDefinition` | Unified constructor for dataset TestDefinition |
7474

7575
---
7676

@@ -635,10 +635,10 @@ All patterns that AST cannot statically resolve are dynamically populated with d
635635

636636
| Component | Status | Description |
637637
|-----------|:------:|------------|
638-
| `resolveDatasetDefinition` || Parses `with data set #N` and `with data set "name"` formats from Teamcity output |
638+
| `DatasetExpander.fromTestOutput` || Parses `with data set #N` and `with data set "name"` formats from Teamcity output |
639+
| `DatasetExpander.fromAnnotations` || Creates dataset children from static analysis labels, ensures consistent format |
639640
| `isDatasetResult` || Used by TestResultObserver to distinguish dataset vs non-dataset results |
640641
| `DatasetObserver` || Dynamically creates dataset child TestItem nodes from Teamcity output after execution |
641-
| `createDatasetDefinition` || Unified constructor for dataset TestDefinition, ensures consistent format |
642642

643643
### Fallback Flow
644644

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ Teamcity 補漏元件:
6767

6868
| 元件 | 說明 |
6969
|------|------|
70-
| `resolveDatasetDefinition` | 解析 Teamcity 輸出中的 `#N``"name"` 格式 |
70+
| `DatasetExpander.fromTestOutput` | 解析 Teamcity 輸出中的 `#N``"name"` 格式 |
71+
| `DatasetExpander.fromAnnotations` | 從靜態分析 labels 建立 dataset children |
7172
| `isDatasetResult` | TestResultObserver 用來分流 dataset vs 非 dataset 結果 |
7273
| `DatasetObserver` | 執行後從 Teamcity 輸出動態建立 dataset 子 TestItem |
73-
| `createDatasetDefinition` | 統一建構 dataset TestDefinition |
7474

7575
---
7676

@@ -635,10 +635,10 @@ testStarted name='it business closed with data set "(|'School|', |'Sunday|')"'
635635

636636
| 元件 | 狀態 | 說明 |
637637
|------|:----:|------|
638-
| `resolveDatasetDefinition` || 解析 Teamcity 輸出中的 `with data set #N``with data set "name"` 格式 |
638+
| `DatasetExpander.fromTestOutput` || 解析 Teamcity 輸出中的 `with data set #N``with data set "name"` 格式 |
639+
| `DatasetExpander.fromAnnotations` || 從靜態分析 labels 建立 dataset children,確保格式一致 |
639640
| `isDatasetResult` || TestResultObserver 用來分流 dataset vs 非 dataset 結果 |
640641
| `DatasetObserver` || 執行後從 Teamcity 輸出動態建立 dataset 子 TestItem 節點 |
641-
| `createDatasetDefinition` || 統一建構 dataset TestDefinition,確保格式一致 |
642642

643643
### 補漏流程
644644

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { datasetIndexed, datasetNamed } from '../../../../utils';
1+
import { datasetExpander } from '../../../../TestParser/DatasetExpander';
22
import type { AstNode, CallNode } from '../../../AstParser/AstNode';
33
import type { Context, Expression } from '../../Expression';
44

@@ -19,7 +19,7 @@ class ArrayCombineExpression implements Expression<string[]> {
1919
return undefined;
2020
}
2121
return [...resolved.values()].map((item, i) =>
22-
typeof item === 'string' ? datasetNamed(item) : datasetIndexed(i),
22+
typeof item === 'string' ? datasetExpander.named(item) : datasetExpander.indexed(i),
2323
);
2424
}
2525
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { datasetIndexed, datasetNamed } from '../../utils';
1+
import { datasetExpander } from '../../TestParser/DatasetExpander';
22
import type { AstNode } from '../AstParser/AstNode';
33
import type { Bindings, Context, Expression } from './Expression';
44
import { arrayCombineExpression } from './Functions/Array/ArrayCombineExpression';
@@ -106,7 +106,7 @@ export function extractLabels(resolved: unknown): string[] | undefined {
106106
return undefined;
107107
}
108108
return [...resolved.keys()].map((key: string) =>
109-
/^\d+$/.test(key) ? datasetIndexed(key) : datasetNamed(key),
109+
/^\d+$/.test(key) ? datasetExpander.indexed(key) : datasetExpander.named(key),
110110
);
111111
}
112112

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { datasetIndexed, datasetNamed } from '../../../utils';
1+
import { datasetExpander } from '../../../TestParser/DatasetExpander';
22
import type { AstNode, IfStatementNode } from '../../AstParser/AstNode';
33
import type { Context } from '../Expression';
44
export const MAX_ITERATIONS = 1000;
@@ -47,9 +47,9 @@ const bodyStatementHandlers: Record<string, (stmt: AstNode, ctx: LoopContext) =>
4747
yield_expression: (stmt, ctx) => {
4848
const resolvedKey = ctx.context.resolve(stmt);
4949
if (resolvedKey !== undefined) {
50-
ctx.labels.push(datasetNamed(String(resolvedKey)));
50+
ctx.labels.push(datasetExpander.named(String(resolvedKey)));
5151
} else {
52-
ctx.labels.push(datasetIndexed(ctx.numericIndex++));
52+
ctx.labels.push(datasetExpander.indexed(ctx.numericIndex++));
5353
}
5454
return undefined;
5555
},

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

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { datasetNamed } from '../../utils';
1+
import { datasetExpander } from '../../TestParser/DatasetExpander';
22
import type { AstNode, CallNode, ExpressionStatementNode } from '../AstParser/AstNode';
33
import { evaluate } from '../Expressions/PhpExpression';
44
import type { PHP } from '../PHP';
@@ -155,9 +155,9 @@ function resolvePestLabels(node: AstNode): string[] {
155155
if (!formatted) {
156156
return [];
157157
}
158-
labels.push(datasetNamed(formatted));
158+
labels.push(datasetExpander.named(formatted));
159159
} else {
160-
labels.push(datasetNamed(`dataset "${key}"`));
160+
labels.push(datasetExpander.named(`dataset "${key}"`));
161161
}
162162
}
163163
return deduplicateLabels(labels);
@@ -169,10 +169,6 @@ function deduplicateLabels(labels: string[]): string[] {
169169
counts.set(label, (counts.get(label) ?? 0) + 1);
170170
}
171171

172-
if ([...counts.values()].every((c) => c === 1)) {
173-
return labels;
174-
}
175-
176172
const indices = new Map<string, number>();
177173
return labels.map((label) => {
178174
if ((counts.get(label) ?? 0) <= 1) {
@@ -208,7 +204,9 @@ function cartesianProduct(datasets: string[][]): string[] {
208204
for (let i = 1; i < datasets.length; i++) {
209205
combinations = combinations.flatMap((combo) => datasets[i].map((v) => [...combo, v]));
210206
}
211-
return combinations.map((combo) => datasetNamed(combo.map((v) => `('${v}')`).join(' / ')));
207+
return combinations.map((combo) =>
208+
datasetExpander.named(combo.map((v) => `('${v}')`).join(' / ')),
209+
);
212210
}
213211

214212
function unwrapClosureBody(node: AstNode): AstNode | undefined {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
import { datasetExpander } from '../../TestParser/DatasetExpander';
12
import type { Annotations } from '../../types';
2-
import { datasetIndexed, datasetNamed } from '../../utils';
33
import type { AstNode, MethodNode } from '../AstParser/AstNode';
44
import type { PHP } from '../PHP';
55
import type { Annotatable, Resolver } from '../types';
@@ -98,12 +98,12 @@ export class TestTagResolver implements Resolver {
9898
const name = attr.args[1];
9999
dataset.push(
100100
typeof name === 'string' && name
101-
? datasetNamed(name)
102-
: datasetIndexed(datasetIndex),
101+
? datasetExpander.named(name)
102+
: datasetExpander.indexed(datasetIndex),
103103
);
104104
datasetIndex++;
105105
} else if (attr.name === 'TestWithJson') {
106-
dataset.push(datasetIndexed(datasetIndex));
106+
dataset.push(datasetExpander.indexed(datasetIndex));
107107
datasetIndex++;
108108
}
109109
}

packages/phpunit/src/ProcessBuilder/FilterStrategy.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { TestIdentifier } from '../TestIdentifier/TestIdentifier';
2+
import { datasetExpander } from '../TestParser/DatasetExpander';
23
import { type TestDefinition, TestType } from '../types';
3-
import { parseDataset } from '../utils';
44

55
abstract class FilterStrategy {
66
constructor(protected testDefinition: TestDefinition) {}
@@ -93,7 +93,7 @@ class DatasetFilterStrategy extends DescribeFilterStrategy {
9393

9494
protected getMethodNamePattern() {
9595
const methodName = (this.testDefinition.methodName ?? '').trim();
96-
const { dataset } = parseDataset(this.testDefinition.id ?? '');
96+
const { dataset } = datasetExpander.parse(this.testDefinition.id ?? '');
9797

9898
const escapedMethod = TestIdentifier.generateSearchText(methodName).replace(/'/g, "\\'");
9999
return `${escapedMethod}${dataset.replace(/[.*+?^${}()|[\]\\/']/g, '\\$&')}`;

packages/phpunit/src/TestCollection/TestCollection.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ import { Minimatch } from 'minimatch';
33
import { URI } from 'vscode-uri';
44
import type { PHPUnitXML, TestDefinition, TestParser, TestSuite } from '../index';
55
import type { TestStarted } from '../TestOutput';
6-
import { resolveDatasetDefinition } from '../TestParser';
76
import { ClassHierarchy } from '../TestParser/ClassHierarchy';
8-
import { parseDataset } from '../utils';
7+
import { datasetExpander } from '../TestParser/DatasetExpander';
98
import { TestStore } from './TestStore';
109

1110
export interface File<T> {
@@ -62,13 +61,13 @@ export class TestCollection {
6261
return undefined;
6362
}
6463

65-
const { parentId } = parseDataset(result.id);
64+
const { parentId } = datasetExpander.parse(result.id);
6665
const parentDef = this.store.getDefinition(parentId);
6766
if (!parentDef) {
6867
return undefined;
6968
}
7069

71-
const childDef = resolveDatasetDefinition(result.name, parentDef);
70+
const childDef = datasetExpander.fromTestOutput(parentDef, result.name);
7271
if (!childDef || this.store.hasDefinition(childDef.id)) {
7372
return undefined;
7473
}

packages/phpunit/src/TestCollection/TestHierarchyBuilder.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { PHPUnitXML } from '../Configuration/PHPUnitXML';
22
import { TestIdentifierFactory } from '../TestIdentifier/TestIdentifierFactory';
3-
import { createDatasetDefinition } from '../TestParser/TestDefinitionBuilder';
3+
import { datasetExpander } from '../TestParser/DatasetExpander';
44
import { type TestDefinition, TestType } from '../types';
55

66
const GROUP_TAG_PREFIX = 'group';
@@ -109,13 +109,11 @@ export abstract class TestHierarchyBuilder<T extends TestTreeItem<T>> {
109109
siblings.push(testItem);
110110
this.testData.set(testItem, test);
111111

112-
const allChildren = [...(test.children ?? [])];
113-
const dataset = test.annotations?.dataset;
114-
if (dataset && dataset.length > 0) {
115-
for (const label of dataset) {
116-
allChildren.push(createDatasetDefinition(test, label));
117-
}
118-
}
112+
const dataset = (test.annotations?.dataset as string[] | undefined) ?? [];
113+
const allChildren = [
114+
...(test.children ?? []),
115+
...datasetExpander.fromAnnotations(test, dataset),
116+
];
119117

120118
if (allChildren.length === 0) {
121119
return;

0 commit comments

Comments
 (0)