chore: filter expectations by the whole test or file name (#9503)

<!-- Thanks for submitting a pull request! Please provide enough
information so that others can review your pull request. -->

**What kind of change does this PR introduce?**

This is a change to a custom mocha runner to look for expectation of the
test case by the whole test name instead of by the part of the name.

**Summary**

Working on integration of the puppeteer expectation file in mozilla repo
and unskipping a lot of tests, I've noticed that some tests get wrong
statuses. For example, a test case with the name `navigation Page.goto
should fail when navigating to bad SSL` got the status of `navigation
Page.goto should fail when navigating to bad SSL after redirects` or
`ElementHandle specs ElementHandle.boundingBox should work` get the
status of `ElementHandle specs ElementHandle.boundingBox should work
with SVG nodes`. So it seems like checking for the whole name of the
test should be safer, but let me know if I'm missing something here.

**Does this PR introduce a breaking change?**
no
This commit is contained in:
Alexandra Borovova 2023-01-13 16:14:37 +01:00 committed by GitHub
parent f59bbf4014
commit 31ff55cc03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 21 deletions

View File

@ -1236,10 +1236,10 @@
"expectations": ["SKIP"] "expectations": ["SKIP"]
}, },
{ {
"testIdPattern": "[page.spec] Page ExecutionContext.queryObjects should work for non-blank page", "testIdPattern": "[page.spec] Page ExecutionContext.queryObjects should work for non-trivial page",
"platforms": ["darwin", "linux", "win32"], "platforms": ["darwin", "linux", "win32"],
"parameters": ["firefox"], "parameters": ["firefox"],
"expectations": ["SKIP"] "expectations": ["FAIL"]
}, },
{ {
"testIdPattern": "[page.spec] Page Page.addStyleTag should throw when added with content to the CSP page", "testIdPattern": "[page.spec] Page Page.addStyleTag should throw when added with content to the CSP page",

View File

@ -30,10 +30,15 @@ const skippedTests: Array<{testIdPattern: string; skip: true}> = process.env[
skippedTests.reverse(); skippedTests.reverse();
function shouldSkipTest(test: Mocha.Test): boolean { function shouldSkipTest(test: Mocha.Test): boolean {
const testId = getTestId(test.file!, test.fullTitle()); const testIdForFileName = getTestId(test.file!);
const testIdForTestName = getTestId(test.file!, test.fullTitle());
// TODO: more efficient lookup. // TODO: more efficient lookup.
const defintion = skippedTests.find(skippedTest => { const defintion = skippedTests.find(skippedTest => {
return testId.startsWith(skippedTest.testIdPattern); return (
'' === skippedTest.testIdPattern ||
testIdForFileName === skippedTest.testIdPattern ||
testIdForTestName === skippedTest.testIdPattern
);
}); });
if (defintion && defintion.skip) { if (defintion && defintion.skip) {
return true; return true;

View File

@ -57,11 +57,11 @@ export function prettyPrintJSON(json: unknown): void {
} }
export function filterByParameters( export function filterByParameters(
expecations: TestExpectation[], expectations: TestExpectation[],
parameters: string[] parameters: string[]
): TestExpectation[] { ): TestExpectation[] {
const querySet = new Set(parameters); const querySet = new Set(parameters);
return expecations.filter(ex => { return expectations.filter(ex => {
return ex.parameters.every(param => { return ex.parameters.every(param => {
return querySet.has(param); return querySet.has(param);
}); });
@ -69,22 +69,20 @@ export function filterByParameters(
} }
/** /**
* The last expectation that matches the startsWith filter wins. * The last expectation that matches an empty string as all tests pattern
* or the name of the file or the whole name of the test the filter wins.
*/ */
export function findEffectiveExpecationForTest( export function findEffectiveExpectationForTest(
expectations: TestExpectation[], expectations: TestExpectation[],
result: MochaTestResult result: MochaTestResult
): TestExpectation | undefined { ): TestExpectation | undefined {
return expectations return expectations
.filter(expecation => { .filter(expectation => {
if ( return (
getTestId(result.file, result.fullTitle).startsWith( '' === expectation.testIdPattern ||
expecation.testIdPattern getTestId(result.file) === expectation.testIdPattern ||
) getTestId(result.file, result.fullTitle) === expectation.testIdPattern
) { );
return true;
}
return false;
}) })
.pop(); .pop();
} }
@ -106,7 +104,7 @@ export function getExpectationUpdates(
const output: RecommendedExpecation[] = []; const output: RecommendedExpecation[] = [];
for (const pass of results.passes) { for (const pass of results.passes) {
const expectation = findEffectiveExpecationForTest(expecations, pass); const expectation = findEffectiveExpectationForTest(expecations, pass);
if (expectation && !expectation.expectations.includes('PASS')) { if (expectation && !expectation.expectations.includes('PASS')) {
output.push({ output.push({
expectation, expectation,
@ -117,7 +115,7 @@ export function getExpectationUpdates(
} }
for (const failure of results.failures) { for (const failure of results.failures) {
const expectation = findEffectiveExpecationForTest(expecations, failure); const expectation = findEffectiveExpectationForTest(expecations, failure);
if (expectation) { if (expectation) {
if ( if (
!expectation.expectations.includes(getTestResultForFailure(failure)) !expectation.expectations.includes(getTestResultForFailure(failure))
@ -156,6 +154,8 @@ export function getTestResultForFailure(
return test.err?.code === 'ERR_MOCHA_TIMEOUT' ? 'TIMEOUT' : 'FAIL'; return test.err?.code === 'ERR_MOCHA_TIMEOUT' ? 'TIMEOUT' : 'FAIL';
} }
export function getTestId(file: string, fullTitle: string): string { export function getTestId(file: string, fullTitle?: string): string {
return `[${getFilename(file)}] ${fullTitle}`; return fullTitle
? `[${getFilename(file)}] ${fullTitle}`
: `[${getFilename(file)}]`;
} }