chore: added * pattern for test expectations (#9870)

2024-06-14 14:02:48 +00:00 · 2023-03-20 10:59:50 +01:00 · 2023-03-20 10:59:50 +01:00 · 9ccde6ebf5
commit 9ccde6ebf5
parent e6ec9c2958
11 changed files with 176 additions and 89 deletions
--- a/.prettierignore
+++ b/.prettierignore
@ -47,3 +47,7 @@ package-lock.json
 test/assets/
 docs/api
 versioned_*/
+
+# Ng-schematics
+/packages/ng-schematics/files/
+/packages/ng-schematics/sandbox/
--- a/packages/ng-schematics/.eslintignore
+++ b/packages/ng-schematics/.eslintignore
@ -1,2 +1,5 @@
 # Ignore File that will be copied to Angular
 /files/
+
+# Ignore sandbox enviroment
+./sandbox/
--- a/test/TestExpectations.json
+++ b/test/TestExpectations.json
@ -12,13 +12,13 @@
    "expectations": ["FAIL", "PASS"]
  },
  {
-    "testIdPattern": "[accessibility.spec]",
+    "testIdPattern": "[accessibility.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP", "TIMEOUT"]
  },
  {
-    "testIdPattern": "[ariaqueryhandler.spec]",
+    "testIdPattern": "[ariaqueryhandler.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -72,7 +72,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[chromiumonly.spec]",
+    "testIdPattern": "[chromiumonly.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -174,7 +174,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[coverage.spec]",
+    "testIdPattern": "[coverage.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -198,7 +198,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[drag-and-drop.spec]",
+    "testIdPattern": "[drag-and-drop.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -390,13 +390,13 @@
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[headful.spec]",
+    "testIdPattern": "[headful.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[idle_override.spec]",
+    "testIdPattern": "[idle_override.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -420,7 +420,7 @@
    "expectations": ["PASS", "FAIL"]
  },
  {
-    "testIdPattern": "[input.spec]",
+    "testIdPattern": "[input.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -954,7 +954,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[oopif.spec]",
+    "testIdPattern": "[oopif.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -1302,7 +1302,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[proxy.spec]",
+    "testIdPattern": "[proxy.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -1314,13 +1314,13 @@
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[requestinterception-experimental.spec]",
+    "testIdPattern": "[requestinterception-experimental.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP", "FAIL"]
  },
  {
-    "testIdPattern": "[requestinterception.spec]",
+    "testIdPattern": "[requestinterception.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP", "FAIL"]
@ -1464,7 +1464,7 @@
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[TargetManager.spec]",
+    "testIdPattern": "[TargetManager.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP", "FAIL"]
@ -1488,7 +1488,7 @@
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[tracing.spec]",
+    "testIdPattern": "[tracing.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -1536,7 +1536,7 @@
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[worker.spec]",
+    "testIdPattern": "[worker.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["firefox"],
    "expectations": ["SKIP"]
@ -1656,7 +1656,7 @@
    "expectations": ["PASS", "FAIL"]
  },
  {
-    "testIdPattern": "",
+    "testIdPattern": "*",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["webDriverBiDi"],
    "expectations": ["SKIP", "TIMEOUT"]
@ -1674,7 +1674,7 @@
    "expectations": ["PASS"]
  },
  {
-    "testIdPattern": "[evaluation.spec]",
+    "testIdPattern": "[evaluation.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["webDriverBiDi"],
    "expectations": ["PASS"]
@ -1782,7 +1782,7 @@
    "expectations": ["SKIP"]
  },
  {
-    "testIdPattern": "[jshandle.spec]",
+    "testIdPattern": "[jshandle.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["webDriverBiDi"],
    "expectations": ["PASS"]
@ -1830,7 +1830,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[queryhandler.spec]",
+    "testIdPattern": "[queryhandler.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["webDriverBiDi"],
    "expectations": ["SKIP", "FAIL"]
@ -1848,7 +1848,7 @@
    "expectations": ["FAIL"]
  },
  {
-    "testIdPattern": "[navigation.spec]",
+    "testIdPattern": "[navigation.spec] *",
    "platforms": ["darwin", "linux", "win32"],
    "parameters": ["webDriverBiDi"],
    "expectations": ["PASS"]
--- a/test/src/click.spec.ts
+++ b/test/src/click.spec.ts
@ -95,7 +95,7 @@ describe('Page.click', function () {
      () => {}
    );
  });
-  it('should click the button after navigation ', async () => {
+  it('should click the button after navigation', async () => {
    const {page, server} = getTestState();

    await page.goto(server.PREFIX + '/input/button.html');
--- a/test/src/page.spec.ts
+++ b/test/src/page.spec.ts
@ -30,7 +30,7 @@ import {
  setupTestBrowserHooks,
  setupTestPageAndContextHooks,
 } from './mocha-utils.js';
-import utils, {attachFrame, waitEvent} from './utils.js';
+import {attachFrame, detachFrame, waitEvent} from './utils.js';

 describe('Page', function () {
  setupTestBrowserHooks();
@ -124,10 +124,7 @@ describe('Page', function () {
    it('should fire when expected', async () => {
      const {page} = getTestState();

-      await Promise.all([
-        page.goto('about:blank'),
-        utils.waitEvent(page, 'load'),
-      ]);
+      await Promise.all([page.goto('about:blank'), waitEvent(page, 'load')]);
    });
  });

@ -1284,11 +1281,11 @@ describe('Page', function () {
      const {page, server} = getTestState();

      await page.goto(server.EMPTY_PAGE);
-      await utils.attachFrame(page, 'frame1', server.EMPTY_PAGE);
+      await attachFrame(page, 'frame1', server.EMPTY_PAGE);
      await page.exposeFunction('compute', function (a: number, b: number) {
        return Promise.resolve(a * b);
      });
-      await utils.detachFrame(page, 'frame1');
+      await detachFrame(page, 'frame1');

      await expect(
        page.evaluate(async function () {
@ -1369,7 +1366,7 @@ describe('Page', function () {
      await page.setUserAgent('foobar');
      const [request] = await Promise.all([
        server.waitForRequest('/empty.html'),
-        utils.attachFrame(page, 'frame1', server.EMPTY_PAGE),
+        attachFrame(page, 'frame1', server.EMPTY_PAGE),
      ]);
      expect(request.headers['user-agent']).toBe('foobar');
    });
--- a/tools/mochaRunner/README.md
+++ b/tools/mochaRunner/README.md
@ -1,23 +1,24 @@
 # Mocha Runner

-Mocha Runner is a test runner on top of mocha. It uses `/test/TestSuites.json` and `/test/TestExpectations.json` files to run mocha tests in multiple configurations and interpret results.
+Mocha Runner is a test runner on top of mocha.
+It uses `/test/TestSuites.json` and `/test/TestExpectations.json` files to run mocha tests in multiple configurations and interpret results.

 ## Running tests for Mocha Runner itself.

-```
+```bash
 npm run build && npx c8 node tools/mochaRunner/lib/test.js
 ```

 ## Running tests using Mocha Runner

-```
+```bash
 npm run build && npm run test
 ```

 By default, the runner runs all test suites applicable to the current platform.
 To pick a test suite, provide the `--test-suite` arguments. For example,

-```
+```bash
 npm run build && npm run test -- --test-suite chrome-headless
 ```

@ -31,17 +32,42 @@ to the given parameter.

 An expectation looks like this:

-```
-  {
+```json
+{
  "testIdPattern": "[accessibility.spec]",
  "platforms": ["darwin", "win32", "linux"],
  "parameters": ["firefox"],
  "expectations": ["SKIP"]
-  }
+}
 ```

-`testIdPattern` defines a string that will be used to prefix-match tests. `platforms` defines the platforms the expectation is for (`or`-logic).
-`parameters` defines the parameters that the test has to match (`and`-logic). `expectations` is the list of test results that are considered to be acceptable.
+| Field           | Description                                                   | Type                                                                                                 | Match Logic |
+| --------------- | ------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | ----------- |
+| `testIdPattern` | Defines the full name (or pattern) to match against test name | string                                                                                               | -           |
+| `platforms`     | Defines the platforms the expectation is for                  | Array<`linux` \| `win32` \|`darwin`>                                                                 | `OR`        |
+| `parameters`    | Defines the parameters that the test has to match             | Array<[ParameterDefinitions](https://github.com/puppeteer/puppeteer/blob/main/test/TestSuites.json)> | `AND`       |
+| `expectations`  | The list of test results that are considered to be acceptable | Array<`PASS` \| `FAIL` \| `TIMEOUT` \| `SKIP`>                                                       | `OR`        |

-Currently, expectations are updated manually. The test runner outputs the suggested changes to the expectation file if the test run does not match
+> Order of defining expectations matters. The latest expectation that is set will take president over earlier ones.
+
+> Adding `SKIP` to `expectations` will prevent the test from running, no matter if there are other expectations.
+
+### Using pattern in `testIdPattern`
+
+Sometimes we want a whole group of test to run. For that we can use a
+pattern to achieve.
+Pattern are defined with the use of `*` (using greedy method).
+
+Examples:
+| Pattern | Description | Example Pattern | Example match |
+|------------------------|---------------------------------------------------------------------------------------------|-----------------------------------|-------------------------------------------------------------------------------------------------------------------------|
+| `*` | Match all tests | - | - |
+| `[test.spec] *` | Matches tests for the given file | `[jshandle.spec] *` | `[jshandle] JSHandle JSHandle.toString should work for primitives` |
+| `[test.spec] <text> *` | Matches tests with for a given test with a specific prefixed test (usually a describe node) | `[page.spec] Page Page.goto *` | `[page.spec] Page Page.goto should work`,<br>`[page.spec] Page Page.goto should work with anchor navigation` |
+| `[test.spec] * <text>` | Matches test with a surfix | `[navigation.spec] * should work` | `[navigation.spec] navigation Page.goto should work`,<br>`[navigation.spec] navigation Page.waitForNavigation should work` |
+
+## Updating Expectations
+
+Currently, expectations are updated manually. The test runner outputs the
+suggested changes to the expectation file if the test run does not match
 expectations.
--- a/tools/mochaRunner/src/interface.ts
+++ b/tools/mochaRunner/src/interface.ts
@ -17,7 +17,7 @@
 import Mocha from 'mocha';
 import commonInterface from 'mocha/lib/interfaces/common';

-import {getTestId} from './utils.js';
+import {testIdMatchesExpectationPattern} from './utils.js';

 type SuiteFunction = ((this: Mocha.Suite) => void) | undefined;
 type ExclusiveSuiteFunction = (this: Mocha.Suite) => void;
@ -28,18 +28,10 @@ const skippedTests: Array<{testIdPattern: string; skip: true}> = process.env[
  ? JSON.parse(process.env['PUPPETEER_SKIPPED_TEST_CONFIG'])
  : [];

-skippedTests.reverse();
-
 function shouldSkipTest(test: Mocha.Test): boolean {
-  const testIdForFileName = getTestId(test.file!);
-  const testIdForTestName = getTestId(test.file!, test.fullTitle());
  // TODO: more efficient lookup.
  const definition = skippedTests.find(skippedTest => {
-    return (
-      '' === skippedTest.testIdPattern ||
-      testIdForFileName === skippedTest.testIdPattern ||
-      testIdForTestName === skippedTest.testIdPattern
-    );
+    return testIdMatchesExpectationPattern(test, skippedTest.testIdPattern);
  });
  if (definition && definition.skip) {
    return true;
--- a/tools/mochaRunner/src/main.ts
+++ b/tools/mochaRunner/src/main.ts
@ -35,6 +35,7 @@ import {
  filterByParameters,
  getExpectationUpdates,
  printSuggestions,
+  RecommendedExpectation,
 } from './utils.js';

 function getApplicableTestSuites(
@ -97,7 +98,7 @@ async function main() {
  }

  let fail = false;
-  const recommendations = [];
+  const recommendations: RecommendedExpectation[] = [];
  try {
    for (const suite of applicableSuites) {
      const parameters = suite.parameters;
@ -105,7 +106,7 @@ async function main() {
      const applicableExpectations = filterByParameters(
        filterByPlatform(expectations, platform),
        parameters
-      );
+      ).reverse();

      // Add more logging when the GitHub Action Debugging option is set
      // https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables
@ -200,17 +201,13 @@ async function main() {
      console.log('Finished', JSON.stringify(parameters));
      try {
        const results = readJSON(tmpFilename) as MochaResults;
-        const recommendation = getExpectationUpdates(
-          results,
-          applicableExpectations,
-          {
+        const updates = getExpectationUpdates(results, applicableExpectations, {
          platforms: [os.platform()],
          parameters,
-          }
-        );
-        if (recommendation.length > 0) {
+        });
+        if (updates.length > 0) {
          fail = true;
-          recommendations.push(...recommendation);
+          recommendations.push(...updates);
        } else {
          console.log('Test run matches expectations');
          continue;
--- a/tools/mochaRunner/src/test.ts
+++ b/tools/mochaRunner/src/test.ts
@ -13,15 +13,15 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-
-import assert from 'assert/strict';
-import test from 'node:test';
+import assert from 'node:assert/strict';
+import {describe, test} from 'node:test';

 import {TestExpectation} from './types.js';
 import {
  filterByParameters,
  getTestResultForFailure,
  isWildCardPattern,
+  testIdMatchesExpectationPattern,
 } from './utils.js';
 import {getFilename, extendProcessEnv} from './utils.js';

@ -67,10 +67,68 @@ test('filterByParameters', () => {
 });

 test('isWildCardPattern', () => {
-  assert.equal(isWildCardPattern(''), true);
+  assert.equal(isWildCardPattern(''), false);
  assert.equal(isWildCardPattern('a'), false);
-  assert.equal(isWildCardPattern('[queryHandler.spec]'), true);
-  assert.equal(isWildCardPattern('[queryHandler.spec] '), true);
-  assert.equal(isWildCardPattern(' [queryHandler.spec] '), true);
-  assert.equal(isWildCardPattern('[queryHandler.spec] test'), 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] Query'), false);
+  assert.equal(isWildCardPattern('[queryHandler.spec] Page *'), true);
+  assert.equal(isWildCardPattern('[queryHandler.spec] Page Page.goto *'), true);
+});
+
+describe('testIdMatchesExpectationPattern', () => {
+  const expectations: Array<[string, boolean]> = [
+    ['', false],
+    ['*', true],
+    ['* should work', true],
+    ['* Page.setContent *', true],
+    ['* should work as expected', false],
+    ['Page.setContent *', false],
+    ['[page.spec]', false],
+    ['[page.spec] *', true],
+    ['[page.spec] Page *', true],
+    ['[page.spec] Page Page.setContent *', true],
+    ['[page.spec] Page Page.setContent should work', true],
+    ['[page.spec] Page * should work', true],
+    ['[page.spec] * Page.setContent *', true],
+    ['[jshandle.spec] *', false],
+    ['[jshandle.spec] JSHandle should work', false],
+  ];
+
+  test('with MochaTest', () => {
+    const test = {
+      title: 'should work',
+      file: 'page.spec.ts',
+      fullTitle() {
+        return 'Page Page.setContent should work';
+      },
+    } as any;
+
+    for (const [pattern, expected] of expectations) {
+      assert.equal(
+        testIdMatchesExpectationPattern(test, pattern),
+        expected,
+        `Expected "${pattern}" to yield "${expected}"`
+      );
+    }
+  });
+  test('with MochaTestResult', () => {
+    const test = {
+      title: 'should work',
+      file: 'page.spec.ts',
+      fullTitle: 'Page Page.setContent should work',
+    } as any;
+
+    for (const [pattern, expected] of expectations) {
+      assert.equal(
+        testIdMatchesExpectationPattern(test, pattern),
+        expected,
+        `Expected "${pattern}" to yield "${expected}"`
+      );
+    }
+  });
 });
--- a/tools/mochaRunner/src/types.ts
+++ b/tools/mochaRunner/src/types.ts
@ -47,6 +47,7 @@ export type TestExpectation = {

 export type MochaTestResult = {
  fullTitle: string;
+  title: string;
  file: string;
  err?: {code: string};
 };
--- a/tools/mochaRunner/src/utils.ts
+++ b/tools/mochaRunner/src/utils.ts
@ -95,28 +95,18 @@ export function findEffectiveExpectationForTest(
  expectations: TestExpectation[],
  result: MochaTestResult
 ): TestExpectation | undefined {
-  return expectations
-    .filter(expectation => {
-      return (
-        '' === expectation.testIdPattern ||
-        getTestId(result.file) === expectation.testIdPattern ||
-        getTestId(result.file, result.fullTitle) === expectation.testIdPattern
-      );
-    })
-    .pop();
+  return expectations.find(expectation => {
+    return testIdMatchesExpectationPattern(result, expectation.testIdPattern);
+  });
 }

-type RecommendedExpectation = {
+export type RecommendedExpectation = {
  expectation: TestExpectation;
  action: 'remove' | 'add' | 'update';
 };

 export function isWildCardPattern(testIdPattern: string): boolean {
-  testIdPattern = testIdPattern.trim();
-  return (
-    testIdPattern === '' ||
-    Boolean(testIdPattern.match(/^\[[a-zA-Z]+\.spec\]$/))
-  );
+  return testIdPattern.includes('*');
 }

 export function getExpectationUpdates(
@ -216,3 +206,22 @@ export function getTestId(file: string, fullTitle?: string): string {
    ? `[${getFilename(file)}] ${fullTitle}`
    : `[${getFilename(file)}]`;
 }
+
+export function testIdMatchesExpectationPattern(
+  test: MochaTestResult | Mocha.Test,
+  pattern: string
+): boolean {
+  const patternRegExString = pattern
+    // Replace `*` with non special character
+    .replace(/\*/g, '--STAR--')
+    // Escape special characters https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#escaping
+    .replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+    // Replace placeholder with greedy match
+    .replace(/--STAR--/g, '(.*)?');
+  // Match beginning and end explicitly
+  const patternRegEx = new RegExp(`^${patternRegExString}$`);
+  const fullTitle =
+    typeof test.fullTitle === 'string' ? test.fullTitle : test.fullTitle();
+
+  return patternRegEx.test(getTestId(test.file ?? '', fullTitle));
+}