test: skip failing on successful retries (#10563)

Co-authored-by: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com>

tools/mochaRunner/src/test.ts

@@ -14,70 +14,84 @@
  * limitations under the License.
  */
 import assert from 'node:assert/strict';
-import {describe, test} from 'node:test';
+import {describe, it} from 'node:test';
 
-import {TestExpectation} from './types.js';
+import {Platform, TestExpectation} from './types.js';
 import {
   filterByParameters,
   getTestResultForFailure,
   isWildCardPattern,
   testIdMatchesExpectationPattern,
+  getExpectationUpdates,
 } from './utils.js';
 import {getFilename, extendProcessEnv} from './utils.js';
 
-void test('extendProcessEnv', () => {
-  const env = extendProcessEnv([{TEST: 'TEST'}, {TEST2: 'TEST2'}]);
-  assert.equal(env['TEST'], 'TEST');
-  assert.equal(env['TEST2'], 'TEST2');
+describe('extendProcessEnv', () => {
+  it('should extend env variables for the subprocess', () => {
+    const env = extendProcessEnv([{TEST: 'TEST'}, {TEST2: 'TEST2'}]);
+    assert.equal(env['TEST'], 'TEST');
+    assert.equal(env['TEST2'], 'TEST2');
+  });
 });
 
-void test('getFilename', () => {
-  assert.equal(getFilename('/etc/test.ts'), 'test');
-  assert.equal(getFilename('/etc/test.js'), 'test');
+describe('getFilename', () => {
+  it('extract filename for a path', () => {
+    assert.equal(getFilename('/etc/test.ts'), 'test');
+    assert.equal(getFilename('/etc/test.js'), 'test');
+  });
 });
 
-void test('getTestResultForFailure', () => {
-  assert.equal(
-    getTestResultForFailure({err: {code: 'ERR_MOCHA_TIMEOUT'}}),
-    'TIMEOUT'
-  );
-  assert.equal(getTestResultForFailure({err: {code: 'ERROR'}}), 'FAIL');
+describe('getTestResultForFailure', () => {
+  it('should get a test result for a mocha failure', () => {
+    assert.equal(
+      getTestResultForFailure({err: {code: 'ERR_MOCHA_TIMEOUT'}}),
+      'TIMEOUT'
+    );
+    assert.equal(getTestResultForFailure({err: {code: 'ERROR'}}), 'FAIL');
+  });
 });
 
-void test('filterByParameters', () => {
-  const expectations: TestExpectation[] = [
-    {
-      testIdPattern:
-        '[oopif.spec] OOPIF "after all" hook for "should keep track of a frames OOP state"',
-      platforms: ['darwin'],
-      parameters: ['firefox', 'headless'],
-      expectations: ['FAIL'],
-    },
-  ];
-  assert.equal(
-    filterByParameters(expectations, ['firefox', 'headless']).length,
-    1
-  );
-  assert.equal(filterByParameters(expectations, ['firefox']).length, 0);
-  assert.equal(
-    filterByParameters(expectations, ['firefox', 'headless', 'other']).length,
-    1
-  );
-  assert.equal(filterByParameters(expectations, ['other']).length, 0);
+describe('filterByParameters', () => {
+  it('should filter a list of expectations by parameters', () => {
+    const expectations: TestExpectation[] = [
+      {
+        testIdPattern:
+          '[oopif.spec] OOPIF "after all" hook for "should keep track of a frames OOP state"',
+        platforms: ['darwin'],
+        parameters: ['firefox', 'headless'],
+        expectations: ['FAIL'],
+      },
+    ];
+    assert.equal(
+      filterByParameters(expectations, ['firefox', 'headless']).length,
+      1
+    );
+    assert.equal(filterByParameters(expectations, ['firefox']).length, 0);
+    assert.equal(
+      filterByParameters(expectations, ['firefox', 'headless', 'other']).length,
+      1
+    );
+    assert.equal(filterByParameters(expectations, ['other']).length, 0);
+  });
 });
 
-void test('isWildCardPattern', () => {
-  assert.equal(isWildCardPattern(''), false);
-  assert.equal(isWildCardPattern('a'), false);
-  assert.equal(isWildCardPattern('*'), true);
+describe('isWildCardPattern', () => {
+  it('should detect if an expectation is a wildcard pattern', () => {
+    assert.equal(isWildCardPattern(''), false);
+    assert.equal(isWildCardPattern('a'), false);
+    assert.equal(isWildCardPattern('*'), true);
 
-  assert.equal(isWildCardPattern('[queryHandler.spec]'), false);
-  assert.equal(isWildCardPattern('[queryHandler.spec] *'), true);
-  assert.equal(isWildCardPattern(' [queryHandler.spec] '), false);
+    assert.equal(isWildCardPattern('[queryHandler.spec]'), false);
+    assert.equal(isWildCardPattern('[queryHandler.spec] *'), true);
+    assert.equal(isWildCardPattern(' [queryHandler.spec] '), false);
 
-  assert.equal(isWildCardPattern('[queryHandler.spec] Query'), false);
-  assert.equal(isWildCardPattern('[queryHandler.spec] Page *'), true);
-  assert.equal(isWildCardPattern('[queryHandler.spec] Page Page.goto *'), true);
+    assert.equal(isWildCardPattern('[queryHandler.spec] Query'), false);
+    assert.equal(isWildCardPattern('[queryHandler.spec] Page *'), true);
+    assert.equal(
+      isWildCardPattern('[queryHandler.spec] Page Page.goto *'),
+      true
+    );
+  });
 });
 
 describe('testIdMatchesExpectationPattern', () => {
@@ -99,7 +113,7 @@ describe('testIdMatchesExpectationPattern', () => {
     ['[jshandle.spec] JSHandle should work', false],
   ];
 
-  void test('with MochaTest', () => {
+  it('with MochaTest', () => {
     const test = {
       title: 'should work',
       file: 'page.spec.ts',
@@ -116,7 +130,8 @@ describe('testIdMatchesExpectationPattern', () => {
       );
     }
   });
-  void test('with MochaTestResult', () => {
+
+  it('with MochaTestResult', () => {
     const test = {
       title: 'should work',
       file: 'page.spec.ts',
@@ -132,3 +147,76 @@ describe('testIdMatchesExpectationPattern', () => {
     }
   });
 });
+
+describe('getExpectationUpdates', () => {
+  it('should generate an update for expectations if a test passed with a fail expectation', () => {
+    const mochaResults = {
+      stats: {tests: 1},
+      pending: [],
+      passes: [
+        {
+          fullTitle: 'Page Page.setContent should work',
+          title: 'should work',
+          file: 'page.spec.ts',
+        },
+      ],
+      failures: [],
+    };
+    const expectations = [
+      {
+        testIdPattern: '[page.spec] Page Page.setContent should work',
+        platforms: ['darwin'] as Platform[],
+        parameters: ['test'],
+        expectations: ['FAIL' as const],
+      },
+    ];
+    const updates = getExpectationUpdates(mochaResults, expectations, {
+      platforms: ['darwin'] as Platform[],
+      parameters: ['test'],
+    });
+    assert.deepEqual(updates, [
+      {
+        action: 'remove',
+        basedOn: {
+          expectations: ['FAIL'],
+          parameters: ['test'],
+          platforms: ['darwin'],
+          testIdPattern: '[page.spec] Page Page.setContent should work',
+        },
+        expectation: {
+          expectations: ['FAIL'],
+          parameters: ['test'],
+          platforms: ['darwin'],
+          testIdPattern: '[page.spec] Page Page.setContent should work',
+        },
+      },
+    ]);
+  });
+
+  it('should not generate an update for successful retries', () => {
+    const mochaResults = {
+      stats: {tests: 1},
+      pending: [],
+      passes: [
+        {
+          fullTitle: 'Page Page.setContent should work',
+          title: 'should work',
+          file: 'page.spec.ts',
+        },
+      ],
+      failures: [
+        {
+          fullTitle: 'Page Page.setContent should work',
+          title: 'should work',
+          file: 'page.spec.ts',
+          err: {code: 'Timeout'},
+        },
+      ],
+    };
+    const updates = getExpectationUpdates(mochaResults, [], {
+      platforms: ['darwin'],
+      parameters: ['test'],
+    });
+    assert.deepEqual(updates, []);
+  });
+});

tools/mochaRunner/src/utils.ts

@@ -134,6 +134,11 @@ export function getExpectationUpdates(
 ): RecommendedExpectation[] {
   const output = new Map<string, RecommendedExpectation>();
 
+  const passesByKey = results.passes.reduce((acc, pass) => {
+    acc.add(getTestId(pass.file, pass.fullTitle));
+    return acc;
+  }, new Set());
+
   for (const pass of results.passes) {
     const expectationEntry = findEffectiveExpectationForTest(
       expectations,
@@ -162,6 +167,9 @@ export function getExpectationUpdates(
   }
 
   for (const failure of results.failures) {
+    if (passesByKey.has(getTestId(failure.file, failure.fullTitle))) {
+      continue;
+    }
     // If an error occurs during a hook
     // the error not have a file associated with it
     if (!failure.file) {