Skip to content

Commit 2b3472e

Browse files
committed
refactor: clean public API and fix bugs in phpunit package
- Remove dead re-exports from Printer barrel (AnsiStyle, DEFAULT_THEME, ColorTheme) - Narrow root barrel to only export TestDefinition and TestType from types.ts - Remove resolveWasmDir from public API - Fix double close event in TestRunner when process spawn fails - Fix XmlElement.getText() return type to string | undefined with call-site guards - Fix TestStore.set() to remove file from old suite when moving to new suite
1 parent 497ee8b commit 2b3472e

8 files changed

Lines changed: 33 additions & 11 deletions

File tree

packages/phpunit/src/Configuration/PHPUnitXML.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@ export class PHPUnitXML {
7878
const name = parent.getAttribute('name') as string;
7979
const prefix = node.getAttribute('prefix');
8080
const suffix = node.getAttribute('suffix');
81+
const text = node.getText() ?? '';
8182

82-
return { tag, name, value: this.resolveToRoot(root, node.getText()), prefix, suffix };
83+
return { tag, name, value: this.resolveToRoot(root, text), prefix, suffix };
8384
};
8485

8586
const excludeCallback = (_tag: string, node: XmlElement, parent: XmlElement) => {
@@ -95,7 +96,7 @@ export class PHPUnitXML {
9596
node.querySelectorAll(type).map((child) => ({
9697
tag: 'exclude',
9798
name,
98-
value: this.resolveToRoot(root, child.getText()),
99+
value: this.resolveToRoot(root, child.getText() ?? ''),
99100
})),
100101
);
101102
};
@@ -166,7 +167,7 @@ export class PHPUnitXML {
166167
for (const parent of this.element.querySelectorAll('phpunit testsuites testsuite')) {
167168
for (const node of parent.querySelectorAll('directory')) {
168169
const dir = node.getText();
169-
if (dir.startsWith('..')) {
170+
if (dir?.startsWith('..')) {
170171
const resolvedAbs = resolve(configRoot, dir);
171172
const configRootAbs = resolve(configRoot);
172173

@@ -195,9 +196,9 @@ export class PHPUnitXML {
195196
const prefix = node.getAttribute('prefix');
196197
const suffix = node.getAttribute('suffix');
197198

198-
return { tag, value: node.getText(), prefix, suffix };
199+
return { tag, value: node.getText() ?? '', prefix, suffix };
199200
},
200-
file: (tag: string, node: XmlElement) => ({ tag, value: node.getText() }),
201+
file: (tag: string, node: XmlElement) => ({ tag, value: node.getText() ?? '' }),
201202
});
202203
}
203204

packages/phpunit/src/Configuration/XmlElement.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ export class XmlElement {
1818
return (this.node[`@_${key}`] as string) ?? undefined;
1919
}
2020

21-
getText(): string {
21+
getText(): string | undefined {
2222
if (typeof this.node === 'string') {
2323
return this.node;
2424
}
2525

26-
return this.node['#text'] as string;
26+
return this.node['#text'] as string | undefined;
2727
}
2828

2929
querySelector(selector: string) {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
export * from './ChainAstParser';
22
export * from './PhpParser/PhpParserAstParser';
33
export * from './TreeSitter/TreeSitterAstParser';
4-
export { initTreeSitter, resolveWasmDir } from './TreeSitter/TreeSitterParser';
4+
export { initTreeSitter } from './TreeSitter/TreeSitterParser';
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
export * from './AnsiStyle';
21
export * from './OutputWriter';
32
export * from './Printer';
43
export * from './PrinterConfig';

packages/phpunit/src/TestCollection/TestStore.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,21 @@ describe('TestStore', () => {
197197
expect(store.size).toBe(2);
198198
});
199199

200+
it('set removes file from old suite when moving to new suite', () => {
201+
const store = new TestStore();
202+
const uri = URI.file('/project/tests/FooTest.php');
203+
204+
store.set('suiteA', uri, makeTests(['t1'], uri.fsPath));
205+
store.set('suiteB', uri, makeTests(['t2'], uri.fsPath));
206+
207+
const files = [...store.gatherFiles()];
208+
const matchingFiles = files.filter((f) => f.uri.toString() === uri.toString());
209+
expect(matchingFiles).toHaveLength(1);
210+
expect(matchingFiles[0].testsuite).toBe('suiteB');
211+
expect(store.getDefinition('t1')).toBeUndefined();
212+
expect(store.getDefinition('t2')).toBeDefined();
213+
});
214+
200215
it('initSuites does not overwrite existing suites', () => {
201216
const store = new TestStore();
202217
const uri = URI.file('/project/tests/FooTest.php');

packages/phpunit/src/TestCollection/TestStore.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ export class TestStore {
2020
}
2121

2222
set(testsuite: string, uri: URI, tests: TestDefinition[]): void {
23-
this.remove(testsuite, uri);
2423
const uriStr = uri.toString();
24+
const oldSuite = this.fileIndex.get(uriStr);
25+
this.remove(oldSuite ?? testsuite, uri);
2526

2627
let suiteFiles = this.suites.get(testsuite);
2728
if (!suiteFiles) {

packages/phpunit/src/TestRunner/TestRunner.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export class TestRunner {
1414
private readonly testOutputParser = new TestOutputParser();
1515
private readonly teamcityPattern = /##teamcity\[/i;
1616
private observers: TestRunnerObserver[] = [];
17+
private closed = false;
1718

1819
constructor() {
1920
this.defaultObserver = createTestRunnerEventProxy();
@@ -34,6 +35,7 @@ export class TestRunner {
3435
}
3536

3637
run(builder: ProcessBuilder) {
38+
this.closed = false;
3739
const process = new TestRunnerProcess(builder);
3840

3941
process.on('start', (builder: ProcessBuilder) => this.emit(TestRunnerEvent.run, builder));
@@ -56,12 +58,16 @@ export class TestRunner {
5658
}
5759

5860
private handleProcessError(err: Error) {
61+
this.closed = true;
5962
const error = err.stack ?? err.message;
6063
this.emit(TestRunnerEvent.error, error);
6164
this.emit(TestRunnerEvent.close, 2);
6265
}
6366

6467
private handleProcessClose(code: number | null, output: string) {
68+
if (this.closed) {
69+
return;
70+
}
6571
const eventName = this.teamcityPattern.test(output)
6672
? TestRunnerEvent.output
6773
: TestRunnerEvent.error;

packages/phpunit/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ export * from './TestCoverage';
66
export * from './TestOutput/types';
77
export * from './TestParser';
88
export * from './TestRunner';
9-
export * from './types';
9+
export { type TestDefinition, TestType } from './types';
1010
export { EOL, parseDataset, semverGte, semverLt } from './utils';

0 commit comments

Comments
 (0)