Clean up mocha test ID helpers [devtools/devtools-frontend : main]
0 views
Skip to first unread message
Eric Leese (Gerrit)
4:36 AM (5 hours ago) 4:36 AM
to Alex Rudenko, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Attention needed from Alex Rudenko
Eric Leese voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Alex Rudenko
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eric Leese (Gerrit)
4:40 AM (5 hours ago) 4:40 AM
to Alex Rudenko, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Attention needed from Alex Rudenko
Eric Leese added 1 comment![]( "Open in Gerrit")
File front_end/testing/MochaHelpers.ts
Line 62, Patchset 2 (Latest):
return testIds.has(testId);Eric Leese . unresolved
This change conflicts with mine, which didn't submit for some reason. Do you want to submit first or combine? All that is needed here is to use the same logic as for isSkipped:
```
return testIds.some(id => testId === id || testId.startsWith(`${id}:`));
```
Related details
Attention is currently required from:
- Alex Rudenko
Submit Requirements:
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
5:01 AM (5 hours ago) 5:01 AM
to Eric Leese, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Alex Rudenko added 1 comment![]( "Open in Gerrit")
File front_end/testing/MochaHelpers.ts
Line 62, Patchset 2 (Latest):
return testIds.has(testId);Eric Leese . resolved
This change conflicts with mine, which didn't submit for some reason. Do you want to submit first or combine? All that is needed here is to use the same logic as for isSkipped:
```
return testIds.some(id => testId === id || testId.startsWith(`${id}:`));
```
Alex Rudenko
Let's land yours first and I will rebase this one.
Related details
Attention set is empty
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eric Leese (Gerrit)
6:38 AM (3 hours ago) 6:38 AM
to Alex Rudenko, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Attention needed from Alex Rudenko
Eric Leese voted and added 1 comment![]( "Open in Gerrit")
Votes added by Eric Leese
| Code-Review | +0 |
1 comment
File front_end/testing/MochaHelpers.ts
Line 62, Patchset 2 (Latest):
return testIds.has(testId);Eric Leese . resolved
Alex RudenkoThis change conflicts with mine, which didn't submit for some reason. Do you want to submit first or combine? All that is needed here is to use the same logic as for isSkipped:
```
return testIds.some(id => testId === id || testId.startsWith(`${id}:`));
```
Let's land yours first and I will rebase this one.
Eric Leese
Done, but you're still going to need to modify this change to include my change!
Related details
Attention is currently required from:
- Alex Rudenko
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
6:43 AM (3 hours ago) 6:43 AM
to Eric Leese, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Attention needed from Eric Leese
Alex Rudenko added 1 comment![]( "Open in Gerrit")
File front_end/testing/MochaHelpers.ts
Line 62, Patchset 2:
return testIds.has(testId);Eric Leese . resolved
Alex RudenkoThis change conflicts with mine, which didn't submit for some reason. Do you want to submit first or combine? All that is needed here is to use the same logic as for isSkipped:
```
return testIds.some(id => testId === id || testId.startsWith(`${id}:`));
```
Eric LeeseLet's land yours first and I will rebase this one.
Done, but you're still going to need to modify this change to include my change!
Attention is currently required from:
- Eric Leese
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Eric Leese (Gerrit)
6:50 AM (3 hours ago) 6:50 AM
to Alex Rudenko, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Attention needed from Alex Rudenko
Eric Leese voted and added 1 comment![]( "Open in Gerrit")
Votes added by Eric Leese
| Code-Review | +1 |
1 comment
File front_end/testing/MochaHelpers.ts
Line 62, Patchset 3 (Latest):
for (const id of testIds) {Eric Leese . resolved
Optional: extract into a function `listContainsTestOrSuite(testIds: Iterable<string>, testId: string)` and use it for isSkipped as well.
Related details
Attention is currently required from:
- Alex Rudenko
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
8:03 AM (2 hours ago) 8:03 AM
to Eric Leese, devtools-fro...@luci-project-accounts.iam.gserviceaccount.com, devtools-rev...@chromium.org
Alex Rudenko voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alex Rudenko
| Auto-Submit | +1 |
| Commit-Queue | +2 |
Eric Leese . resolved
Optional: extract into a function `listContainsTestOrSuite(testIds: Iterable<string>, testId: string)` and use it for isSkipped as well.
Submit Requirements:
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
devtools-frontend-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
8:34 AM (1 hour ago) 8:34 AM
to Alex Rudenko, Eric Leese, devtools-rev...@chromium.org
devtools-fro...@luci-project-accounts.iam.gserviceaccount.com submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: front_end/testing/MochaHelpers.ts
Insertions: 11, Deletions: 7.
@@ -39,6 +39,15 @@
skippedTests?: readonly string[];
}
+export function listContainsTestOrSuite(list: Iterable<string>, testId: string): boolean {
+ for (const item of list) {
+ if (testId === item || testId.startsWith(`${item}:`)) {
+ return true;
+ }
+ }
+ return false;
+}
+
export function pruneSuite(
suite: Mocha.Suite,
testIdMap: ReadonlyMap<Mocha.Test, string>,
@@ -52,19 +61,14 @@
if (!testId) {
return false;
}
- const isSkipped = skippedTests.some(skippedTest => testId === skippedTest || testId.startsWith(`${skippedTest}:`));
+ const isSkipped = listContainsTestOrSuite(skippedTests, testId);
if (isSkipped) {
test.pending = true;
}
if (!testIds || testIds.size === 0) {
return true;
}
- for (const id of testIds) {
- if (testId === id || testId.startsWith(`${id}:`)) {
- return true;
- }
- }
- return false;
+ return listContainsTestOrSuite(testIds, testId);
});
for (const subSuite of suite.suites) {
```
```
The name of the file: front_end/testing/MochaHelpers.test.ts
Insertions: 55, Deletions: 21.
@@ -9,6 +9,7 @@
checkForDuplicateTests,
createTestIdMap,
duplicateTests,
+ listContainsTestOrSuite,
pruneSuite,
} from './MochaHelpers.js';
@@ -29,8 +30,8 @@
describe('MochaHelpers', () => {
describe('createTestIdMap', () => {
it('creates a map of Mocha.Test to computed test ID for all tests in suites', () => {
- const test1 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
- const test2 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'sub suite', 'test 2']});
+ const test1 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test2 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'sub suite', 'test 2']});
const subSuite: Mocha.Suite = {
tests: [test2],
suites: [],
@@ -41,8 +42,8 @@
} as unknown as Mocha.Suite;
const map = createTestIdMap(rootSuite);
- assert.strictEqual(map.get(test1), '/base/test.ts:suite:test_1');
- assert.strictEqual(map.get(test2), '/base/test.ts:suite:sub_suite:test_2');
+ assert.strictEqual(map.get(test1), 'base/test.ts:suite:test_1');
+ assert.strictEqual(map.get(test2), 'base/test.ts:suite:sub_suite:test_2');
assert.strictEqual(map.size, 2);
});
@@ -59,33 +60,33 @@
describe('checkForDuplicateTests', () => {
it('passes when there are no duplicates', () => {
- const test1 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
- const test2 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 2']});
+ const test1 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test2 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 2']});
const map = new Map<Mocha.Test, string>([
- [test1, '/base/test.ts:suite:test_1'],
- [test2, '/base/test.ts:suite:test_2'],
+ [test1, 'base/test.ts:suite:test_1'],
+ [test2, 'base/test.ts:suite:test_2'],
]);
assert.doesNotThrow(() => checkForDuplicateTests(map));
});
it('throws when duplicate tests are found', () => {
- const test1 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
- const test2 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test1 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test2 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
const map = new Map<Mocha.Test, string>([
- [test1, '/base/test.ts:suite:test_1'],
- [test2, '/base/test.ts:suite:test_1'],
+ [test1, 'base/test.ts:suite:test_1'],
+ [test2, 'base/test.ts:suite:test_1'],
]);
- assert.throws(() => checkForDuplicateTests(map), /Duplicate test \/base\/test\.ts:suite:test_1/);
+ assert.throws(() => checkForDuplicateTests(map), /Duplicate test base\/test\.ts:suite:test_1/);
});
});
describe('pruneSuite', () => {
it('recursively filters tests in suites using testIds', () => {
- const test1 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
- const test2 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'sub suite', 'test 2']});
- const test3 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'sub suite', 'test 3']});
+ const test1 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test2 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'sub suite', 'test 2']});
+ const test3 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'sub suite', 'test 3']});
const subSuite: Mocha.Suite = {
tests: [test2, test3],
@@ -99,7 +100,7 @@
const map = createTestIdMap(rootSuite);
pruneSuite(rootSuite, map, {
- testIds: new Set(['/base/test.ts:suite:sub_suite:test_2']),
+ testIds: new Set(['base/test.ts:suite:sub_suite:test_2']),
});
assert.deepEqual(rootSuite.tests, []);
@@ -107,8 +108,8 @@
});
it('marks skipped tests as pending (by exact match or prefix)', () => {
- const test1 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
- const test2 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'sub suite', 'test 2']});
+ const test1 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test2 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'sub suite', 'test 2']});
const subSuite: Mocha.Suite = {
tests: [test2],
@@ -122,7 +123,7 @@
const map = createTestIdMap(rootSuite);
pruneSuite(rootSuite, map, {
- skippedTests: ['/base/test.ts:suite:test_1', '/base/test.ts:suite:sub_suite'],
+ skippedTests: ['base/test.ts:suite:test_1', 'base/test.ts:suite:sub_suite'],
});
assert.isTrue(test1.pending);
@@ -132,7 +133,7 @@
});
it('keeps all tests when testIds is empty', () => {
- const test1 = createMockTest({file: '/base/test.ts', titlePath: ['Suite', 'test 1']});
+ const test1 = createMockTest({file: 'base/test.ts', titlePath: ['Suite', 'test 1']});
const suite: Mocha.Suite = {
tests: [test1],
suites: [],
@@ -167,4 +168,37 @@
assert.lengthOf(suite.tests, 3);
});
});
+
+ describe('listContainsTestOrSuite', () => {
+ it('returns true when exact test ID is present in an array', () => {
+ const list = ['base/test.ts:suite:test_1', 'base/test.ts:suite:test_2'];
+ assert.isTrue(listContainsTestOrSuite(list, 'base/test.ts:suite:test_1'));
+ });
+
+ it('returns true when exact test ID is present in a set', () => {
+ const set = new Set(['base/test.ts:suite:test_1', 'base/test.ts:suite:test_2']);
+ assert.isTrue(listContainsTestOrSuite(set, 'base/test.ts:suite:test_1'));
+ });
+
+ it('returns true when a suite prefix of test ID is present', () => {
+ const list = ['base/test.ts:suite'];
+ assert.isTrue(listContainsTestOrSuite(list, 'base/test.ts:suite:test_1'));
+ assert.isTrue(listContainsTestOrSuite(list, 'base/test.ts:suite:sub_suite:test_2'));
+ });
+
+ it('returns false when test ID is not present and no suite prefix matches', () => {
+ const list = ['base/test.ts:suite:test_2', 'base/other.ts:suite'];
+ assert.isFalse(listContainsTestOrSuite(list, 'base/test.ts:suite:test_1'));
+ });
+
+ it('returns false when item in list is only a prefix without colon separator', () => {
+ const list = ['base/test.ts:suite'];
+ assert.isFalse(listContainsTestOrSuite(list, 'base/test.ts:suite_extra:test_1'));
+ });
+
+ it('returns false when list is empty', () => {
+ assert.isFalse(listContainsTestOrSuite([], 'base/test.ts:suite:test_1'));
+ assert.isFalse(listContainsTestOrSuite(new Set(), 'base/test.ts:suite:test_1'));
+ });
+ });
});
```
Change information
Commit message:
Clean up mocha test ID helpers
- deduplicate between e2e and unit tests
- create dedicated helpers for various steps
Bug: none
Change-Id: I25fbb7ca80fcc792e18d7511eb813aba7cd3e9d0
Auto-Submit: Alex Rudenko <alexr...@chromium.org>
Reviewed-by: Eric Leese <le...@chromium.org>
Commit-Queue: Alex Rudenko <alexr...@chromium.org>
Files:
- M front_end/testing/BUILD.gn
- A front_end/testing/MochaHelpers.test.ts
- M front_end/testing/MochaHelpers.ts
- M test/shared/run-mocha.ts
- M test/unit/mocha-adapter-browser.ts
Change size: L
Delta: 5 files changed, 302 insertions(+), 74 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Eric Leese
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages