Skip to content

Commit c235d1a

Browse files
authored
Construct minimal regex when running unit tests in parallel (#4830)
1 parent 28550f9 commit c235d1a

3 files changed

Lines changed: 197 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ Bug Fixes:
7373
- Fix diagnostics to handle when there isn't a command in the error output. [PR #4765](https://github.com/microsoft/vscode-cmake-tools/pull/4765)
7474
- Fix bug in which running "CMake: Build" would always run "CMake: Clean Rebuild" when `cmake.buildTask` is enabled [#4421](https://github.com/microsoft/vscode-cmake-tools/issues/4421) [@RedSkittleFox](https://github.com/RedSkittleFox)
7575
- Fix issue with hover provider not checking for undefined. [#4812](https://github.com/microsoft/vscode-cmake-tools/issues/4812)
76+
- Fix bug in which CTest is unable to run large amount of tests in parallel due to regex exceeding command line length limits [#4829](https://github.com/microsoft/vscode-cmake-tools/issues/4829) [@theuke](https://github.com/theuke)
7677

7778
## 1.22.28
7879

src/ctest.ts

Lines changed: 105 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,96 @@ export function searchOutputForFailures(patterns: FailurePatternsConfig, output:
166166
return messages;
167167
}
168168

169+
/**
170+
* Finds the minimal regex fragments to match targets from the superset.
171+
* If a target is a prefix of a forbidden string, it returns an exact match.
172+
* Otherwise, it returns a minimal prefix.
173+
*
174+
* @param superset : the list of all individual test names (individual and or group names)
175+
* @param targets : the list of test names (individual and or group names) we want to match.
176+
* @returns : the list of regex fragments to match the targets without matching any other test from the superset
177+
*/
178+
export function getMinimalRegexFragments(superset: string[], targets: string[]): string[] {
179+
interface TrieNode {
180+
children: Map<string, TrieNode>;
181+
forbiddenCount: number;
182+
}
183+
184+
const targetSet = new Set(targets);
185+
const forbidden = superset.filter(s => !targetSet.has(s));
186+
187+
if (targets.length === 0) {
188+
return [];
189+
}
190+
if (forbidden.length === 0) {
191+
return ["^."];
192+
}
193+
194+
const root: TrieNode = { children: new Map(), forbiddenCount: 0 };
195+
196+
// 1. Build Trie with forbidden strings
197+
for (const s of forbidden) {
198+
let current = root;
199+
for (const char of s) {
200+
if (!current.children.has(char)) {
201+
current.children.set(char, { children: new Map(), forbiddenCount: 0 });
202+
}
203+
current = current.children.get(char)!;
204+
current.forbiddenCount++;
205+
}
206+
}
207+
208+
const fragmentSet = new Set<string>();
209+
210+
// 2. Identify the optimal regex fragment for each target
211+
for (const target of targets) {
212+
let current = root;
213+
let prefix = "";
214+
let foundUnique = false;
215+
216+
for (const char of target) {
217+
prefix += char;
218+
const next = current.children.get(char);
219+
220+
if (!next || next.forbiddenCount === 0) {
221+
// Safe unique prefix found
222+
fragmentSet.add(`^${util.escapeStringForRegex(prefix)}`);
223+
foundUnique = true;
224+
break;
225+
}
226+
current = next;
227+
}
228+
229+
// 3. The "Swallowed" Target Case
230+
// Target is a prefix of a forbidden string (e.g., "Test" vs "Test.1")
231+
if (!foundUnique) {
232+
fragmentSet.add(`^${util.escapeStringForRegex(target)}$`);
233+
}
234+
}
235+
236+
// 4. Remove redundant fragments
237+
// Sort by length: "Test\." (length 6) comes before "Test\.x\.1" (length 10)
238+
const sorted = Array.from(fragmentSet).sort((a, b) => a.length - b.length);
239+
const minimalFragments: string[] = [];
240+
241+
for (const fragment of sorted) {
242+
// If we already have a prefix that covers this fragment, skip it.
243+
// Note: 'existing' only matches 'fragment' if it's a true prefix (no $ anchor)
244+
const isRedundant = minimalFragments.some(existing => {
245+
if (existing.endsWith('$')) {
246+
return false; // Exact matches can't cover other things
247+
}
248+
return fragment.startsWith(existing);
249+
});
250+
251+
if (!isRedundant) {
252+
minimalFragments.push(fragment);
253+
}
254+
}
255+
256+
return minimalFragments;
257+
}
258+
169259
function matchToTestMessage(pat: FailurePattern, match: RegExpMatchArray): vscode.TestMessage {
170260
const file = match[pat.file as number];
171261
const line = pat.line ? parseLineMatch(match[pat.line]) : 0;
@@ -442,7 +532,8 @@ export class CTestDriver implements vscode.Disposable {
442532
const ctestArgs = await this.getCTestArgs(driver, customizedTask, testPreset) || [];
443533
if (testsToRun && testsToRun.length > 0) {
444534
ctestArgs.push("-R");
445-
const testsNamesRegex = testsToRun.map(t => `^${util.escapeStringForRegex(t)}\$`).join('|');
535+
const superset = this.getTestNames() || [];
536+
const testsNamesRegex = getMinimalRegexFragments(superset, testsToRun).join('|');
446537
ctestArgs.push(testsNamesRegex);
447538
}
448539

@@ -613,7 +704,8 @@ export class CTestDriver implements vscode.Disposable {
613704

614705
run.started(test);
615706

616-
const _ctestArgs = driver.ctestArgs.concat('-R', `^${util.escapeStringForRegex(test.id)}\$`);
707+
const superset = this.getTestNames() || [];
708+
const _ctestArgs = driver.ctestArgs.concat('-R', getMinimalRegexFragments(superset, [test.id]).join('|'));
617709

618710
const testResults = await this.runCTestImpl(driver.driver, driver.ctestPath, _ctestArgs, cancellation, customizedTask, consumer);
619711

@@ -651,21 +743,21 @@ export class CTestDriver implements vscode.Disposable {
651743
// then there may be a scenario when the user requested only a subset of tests to be ran.
652744
// In this case, we should specifically use the -R flag to select the exact tests.
653745
// Otherwise, we can leave it to the -T flag to run all tests.
746+
let targetTests: vscode.TestItem[] | undefined;
654747
if (entryPoint === RunCTestHelperEntryPoint.TestExplorer && testExplorer && this._tests && this._tests.tests.length !== driver.tests.length) {
655-
uniqueCtestArgs.push("-R");
656-
const testsNamesRegex = driver.tests.map(t => {
657-
run.started(t);
658-
return `^${util.escapeStringForRegex(t.id)}\$`;
659-
}).join('|');
660-
uniqueCtestArgs.push(testsNamesRegex);
748+
targetTests = driver.tests;
661749
} else if (testsToRun && testsToRun.length > 0) {
750+
targetTests = driver.tests.filter(t => testsToRun.includes(t.id));
751+
}
752+
753+
if (targetTests) {
662754
uniqueCtestArgs.push("-R");
663-
const tests = driver.tests.filter(t => testsToRun.includes(t.id));
664-
const testsNamesRegex = tests.map(t => {
755+
const superset = this.getTestNames() || [];
756+
const targets = targetTests.map(t => {
665757
run.started(t);
666-
return `^${util.escapeStringForRegex(t.id)}\$`;
667-
}).join('|');
668-
uniqueCtestArgs.push(testsNamesRegex);
758+
return t.id;
759+
});
760+
uniqueCtestArgs.push(getMinimalRegexFragments(superset, targets).join('|'));
669761
}
670762

671763
const testResults = await this.runCTestImpl(uniqueDriver, uniqueCtestPath, uniqueCtestArgs, cancellation, customizedTask, consumer);

test/unit-tests/ctest.test.ts

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readTestResultsFile, searchOutputForFailures } from "@cmt/ctest";
1+
import { readTestResultsFile, searchOutputForFailures, getMinimalRegexFragments } from "@cmt/ctest";
22
import { expect, getTestResourceFilePath } from "@test/util";
33
import { TestMessage } from "vscode";
44

@@ -111,4 +111,94 @@ suite('CTest test', () => {
111111
expect(tm.expectedOutput).to.eq(expected);
112112
expect(tm.actualOutput).to.eq(actual);
113113
}
114+
115+
suite('getMinimalRegexFragments', () => {
116+
test('no targets', () => {
117+
const result = getMinimalRegexFragments(['A', 'B', 'C'], []);
118+
expect(result).to.deep.eq([]);
119+
});
120+
121+
test('no forbidden strings', () => {
122+
const result = getMinimalRegexFragments(['A', 'B', 'C'], ['A', 'B', 'C']);
123+
expect(result).to.deep.eq(['^.']);
124+
});
125+
126+
test('unique prefixes map correctly', () => {
127+
const superset = ['Test1', 'Test2', 'OtherTest'];
128+
// Target is a unique subset
129+
const result = getMinimalRegexFragments(superset, ['Test1', 'OtherTest']);
130+
// Test1 -> T e s t 1 (at '1', no other forbidden string shares this prefix since Test2 diverges at 1)
131+
// OtherTest -> O (matches nothing forbidden immediately)
132+
// wait, forbidden is ['Test2']
133+
// For 'Test1': prefix 'Test1' -> forbidden count 0. Wait, 'T' matches 'Test2', 'e', 's', 't', '1'. 'Test1' is the prefix!
134+
expect(result.length).to.eq(2);
135+
expect(result).to.include('^Test1');
136+
expect(result).to.include('^O');
137+
});
138+
139+
test('swallowed target case (prefix of forbidden string)', () => {
140+
const superset = ['Test', 'Test.1', 'Test.2'];
141+
const targets = ['Test'];
142+
// Since 'Test' is a prefix of 'Test.1', it never finds a prefix that has forbiddenCount 0.
143+
// It parses all characters of 'Test' and then uses an end anchor.
144+
const result = getMinimalRegexFragments(superset, targets);
145+
expect(result).to.deep.eq(['^Test$']);
146+
});
147+
148+
test('handles regex special characters properly', () => {
149+
const superset = ['Test[A]', 'Test[B]'];
150+
const targets = ['Test[A]'];
151+
// Forbidden is ['Test[B]']
152+
// 'Test[' is shared. 'Test[A' is unique.
153+
const result = getMinimalRegexFragments(superset, targets);
154+
expect(result).to.deep.eq(['^Test\\[A']);
155+
});
156+
157+
test('removes redundant fragments', () => {
158+
// Targets: ['A', 'AB']
159+
// Forbidden: ['B']
160+
// 'A' -> 'A', AB -> 'A'
161+
// "A" covers "AB", so "AB" is redundant.
162+
const result = getMinimalRegexFragments(['A', 'AB', 'B'], ['A', 'AB']);
163+
// forbidden: 'B'. root has 'B'.
164+
// 'A' char 'A' -> forbidden 0 -> 'A'
165+
// 'AB' char 'A' -> forbidden 0 -> 'A'
166+
// result ['^A']
167+
expect(result).to.deep.eq(['^A']);
168+
});
169+
170+
test('complex edge cases: nested suites, swallowed prefixes, special characters', () => {
171+
const superset = [
172+
'Suite', // Target: gets swallowed by prefix sharing with forbidden
173+
'Suite.Test1', // Target: logically nested
174+
'Suite.Test2', // Target: logically nested
175+
'Suite.Test3', // Forbidden
176+
'OtherSuite.Test1', // Target
177+
'OtherSuite.Test2', // Target
178+
'O[ther]', // Forbidden: share 'O' prefix
179+
'A+B.Test', // Target: gets swallowed + special chars
180+
'A+B.Test2' // Forbidden
181+
];
182+
183+
const targets = [
184+
'Suite',
185+
'Suite.Test1',
186+
'Suite.Test2',
187+
'OtherSuite.Test1',
188+
'OtherSuite.Test2',
189+
'A+B.Test'
190+
];
191+
192+
const result = getMinimalRegexFragments(superset, targets);
193+
194+
expect(result).to.have.members([
195+
'^Suite$',
196+
'^Suite\\.Test1',
197+
'^Suite\\.Test2',
198+
'^Ot',
199+
'^A\\+B\\.Test$'
200+
]);
201+
expect(result.length).to.eq(5);
202+
});
203+
});
114204
});

0 commit comments

Comments
 (0)