From c7b2fc27cb58b4e3bc7a58a28709e152945d8d86 Mon Sep 17 00:00:00 2001 From: ryanrosello-og <50574915+ryanrosello-og@users.noreply.github.com> Date: Sat, 11 Nov 2023 13:58:21 +1000 Subject: [PATCH] Fix incorrect stats calculations (#68) * WIP: fix stats * remove separate flakey config * shuffle expected stats * add config for nyc * update node version gh wf to 18.18.2 --- .github/workflows/publish.yml | 4 +- .gitignore | 1 + README.md | 2 - package.json | 13 +++++- playwright.config.ts | 3 ++ src/ResultsParser.ts | 84 ++++++--------------------------- src/SlackReporter.ts | 9 +--- src/index.ts | 9 ---- tests/ResultsParser.spec.ts | 88 +++++++++++------------------------ 9 files changed, 62 insertions(+), 151 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index ea43933..b4f9561 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -10,10 +10,10 @@ jobs: runs-on: ubuntu-20.04 steps: - uses: actions/checkout@v1 - - name: install node v16.13.1 + - name: install node v18.18.2 uses: actions/setup-node@v1 with: - node-version: 16.13.1 + node-version: 18.18.2 - name: yarn install run: yarn install - name: build diff --git a/.gitignore b/.gitignore index 2769e73..719ca27 100644 --- a/.gitignore +++ b/.gitignore @@ -113,3 +113,4 @@ dist/ # VS Code files .vscode bot +playwright-report/ \ No newline at end of file diff --git a/README.md b/README.md index 0524faf..648dffd 100644 --- a/README.md +++ b/README.md @@ -179,8 +179,6 @@ A function that returns a layout object, this configuration is optional. See se Same as **layout** above, but asynchronous in that it returns a promise. ### **maxNumberOfFailuresToShow** Limits the number of failures shown in the Slack message, defaults to 10. -### **separateFlaky** -Don't consider flaky tests as passed, instead output them separately. Flaky tests are tests that passed but not on the first try. This defaults to `false` but it is highly recommended that you switch this on. ### **slackOAuthToken** Instead of providing an environment variable `SLACK_BOT_USER_OAUTH_TOKEN` you can specify the token in the config in the `slackOAuthToken` field. ### **slackLogLevel** (default LogLevel.DEBUG) diff --git a/package.json b/package.json index 9d4d01f..28bbd6e 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "lint-fix": "npx eslint . --ext .ts --fix" }, "name": "playwright-slack-report", - "version": "1.1.24", + "version": "1.1.25", "main": "index.js", "types": "dist/index.d.ts", "repository": "git@github.com:ryanrosello-og/playwright-slack-report.git", @@ -44,5 +44,14 @@ "report", "playwright", "typescript" - ] + ], + "nyc": { + "all": true, + "include": [ + "src/**/*.ts" + ], + "exclude": [ + "./src/custom_block/my_block.ts" + ] + } } diff --git a/playwright.config.ts b/playwright.config.ts index ee0a804..164ff6d 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -7,6 +7,8 @@ require('dotenv').config(); const config: PlaywrightTestConfig = { forbidOnly: !!process.env.CI, + workers: 4, + retries: 1, use: { trace: 'on', screenshot: 'on', @@ -45,6 +47,7 @@ const config: PlaywrightTestConfig = { }, ], ['dot'], + ['html'], ], }; export default config; diff --git a/src/ResultsParser.ts b/src/ResultsParser.ts index 8751651..9150544 100644 --- a/src/ResultsParser.ts +++ b/src/ResultsParser.ts @@ -5,8 +5,9 @@ /* eslint-disable class-methods-use-this */ /* eslint-disable no-param-reassign */ +import { TestCase } from '@playwright/test/reporter'; import { - failure, flaky, pass, SummaryResults, + failure, SummaryResults, } from '.'; /* eslint-disable no-restricted-syntax */ @@ -40,39 +41,29 @@ export type testSuite = { export default class ResultsParser { private result: testSuite[]; - private separateFlakyTests: boolean; - - constructor( - options: { separateFlakyTests: boolean } = { separateFlakyTests: false }, - ) { + constructor() { this.result = []; - this.separateFlakyTests = options.separateFlakyTests; } - async getParsedResults(): Promise { + async getParsedResults(allTests: Array): Promise { const failures = await this.getFailures(); - const flakes = await this.getFlakes(); - let passes = await this.getPasses(); - - if (this.separateFlakyTests) { - passes = this.doSeparateFlakyTests(passes, flakes); - } - + // use Playwright recommended way of extracting test stats: + // https://github.com/microsoft/playwright/issues/27498#issuecomment-1766766335 + const stats = { + expected: 0, skipped: 0, unexpected: 0, flaky: 0, + }; + // eslint-disable-next-line no-plusplus + for (const test of allTests) ++stats[test.outcome()]; const summary: SummaryResults = { - passed: passes.length, - failed: failures.length, - flaky: this.separateFlakyTests ? flakes.length : undefined, - skipped: 0, + passed: stats.expected, + failed: stats.unexpected, + flaky: stats.flaky, + skipped: stats.skipped, failures, tests: [], }; for (const suite of this.result) { summary.tests = summary.tests.concat(suite.testSuite.tests); - for (const test of suite.testSuite.tests) { - if (test.status === 'skipped') { - summary.skipped += 1; - } - } } return summary; } @@ -95,35 +86,6 @@ export default class ResultsParser { return failures; } - async getFlakes(): Promise> { - const flaky: Array = []; - for (const suite of this.result) { - for (const test of suite.testSuite.tests) { - if (test.status === 'passed' && test.retry > 0) { - flaky.push({ - test: ResultsParser.getTestName(test), - retry: test.retry, - }); - } - } - } - return flaky; - } - - async getPasses(): Promise> { - const passes: Array = []; - for (const suite of this.result) { - for (const test of suite.testSuite.tests) { - if (test.status === 'passed') { - passes.push({ - test: ResultsParser.getTestName(test), - }); - } - } - } - return passes; - } - static getTestName(failedTest: any) { const testName = failedTest.name; if (failedTest.browser && failedTest.projectName) { @@ -233,20 +195,4 @@ export default class ResultsParser { browser: '', }; } - - /** removes tests from the passed array that only passed on a retry (flaky). - * Does not modify param passed, returns a new passed array. */ - doSeparateFlakyTests(passes: Array, flakes: Array) { - const _passes: Map = new Map(); - - for (const pass of passes) { - _passes.set(pass.test, pass); - } - - for (const flake of flakes) { - _passes.delete(flake.test); - } - - return [..._passes.values()]; - } } diff --git a/src/SlackReporter.ts b/src/SlackReporter.ts index 5d7a65c..b4deae4 100644 --- a/src/SlackReporter.ts +++ b/src/SlackReporter.ts @@ -45,8 +45,6 @@ class SlackReporter implements Reporter { private suite!: Suite; - private separateFlaky: boolean = false; - logs: string[] = []; onBegin(fullConfig: FullConfig, suite: Suite): void { @@ -79,11 +77,8 @@ class SlackReporter implements Reporter { this.showInThread = slackReporterConfig.showInThread || false; this.slackLogLevel = slackReporterConfig.slackLogLevel || LogLevel.DEBUG; this.proxy = slackReporterConfig.proxy || undefined; - this.separateFlaky = slackReporterConfig.separateFlaky || false; } - this.resultsParser = new ResultsParser({ - separateFlakyTests: this.separateFlaky, - }); + this.resultsParser = new ResultsParser(); } // eslint-disable-next-line class-methods-use-this, no-unused-vars @@ -98,7 +93,7 @@ class SlackReporter implements Reporter { return; } - const resultSummary = await this.resultsParser.getParsedResults(); + const resultSummary = await this.resultsParser.getParsedResults(this.suite.allTests()); resultSummary.meta = this.meta; const maxRetry = Math.max(...resultSummary.tests.map((o) => o.retry)); if ( diff --git a/src/index.ts b/src/index.ts index 47ed02e..25b2c49 100644 --- a/src/index.ts +++ b/src/index.ts @@ -28,12 +28,3 @@ export type failure = { test: string; failureReason: string; }; - -export type flaky = { - test: string; - retry: number; -}; - -export type pass = { - test: string; -}; diff --git a/tests/ResultsParser.spec.ts b/tests/ResultsParser.spec.ts index e63efd8..640e560 100644 --- a/tests/ResultsParser.spec.ts +++ b/tests/ResultsParser.spec.ts @@ -1,4 +1,5 @@ import { expect, test as base } from '@playwright/test'; +import { TestCase } from '@playwright/test/reporter'; import ResultsParser from '../src/ResultsParser'; const test = base.extend<{ testData: any }>({ @@ -385,7 +386,7 @@ test.describe('ResultsParser', () => { test('determines correct browser based on project config', async ({ testData, }) => { - const resultsParser = new ResultsParser({ separateFlakyTests: false }); + const resultsParser = new ResultsParser(); resultsParser.addTestResult( testData.suites[0].suites[0].title, testData.suites[0].suites[0].tests[0], @@ -400,23 +401,44 @@ test.describe('ResultsParser', () => { }, ], ); - const results = await resultsParser.getParsedResults(); + const results = await resultsParser.getParsedResults([]); expect(results.tests[0].projectName).toEqual('playwright-slack-report'); expect(results.tests[0].browser).toEqual('chrome'); }); test('parses results successfully', async ({ testData }) => { - const resultsParser = new ResultsParser({ separateFlakyTests: false }); + const resultsParser = new ResultsParser(); resultsParser.addTestResult( testData.suites[0].suites[0].title, testData.suites[0].suites[0].tests[0], [], ); - const results = await resultsParser.getParsedResults(); + const testA: TestCase = { + expectedStatus: 'failed', + ok(): boolean { + throw new Error('Function not implemented.'); + }, + outcome(): 'skipped' | 'expected' | 'unexpected' | 'flaky' { + return 'unexpected'; + }, + titlePath(): string[] { + throw new Error('Function not implemented.'); + }, + annotations: [], + id: '', + location: undefined, + parent: undefined, + repeatEachIndex: 0, + results: [], + retries: 0, + timeout: 0, + title: '', + }; + const results = await resultsParser.getParsedResults([testA]); expect(results).toEqual({ passed: 0, - failed: 0, - flaky: undefined, + failed: 1, + flaky: 0, skipped: 0, failures: [], tests: [ @@ -467,60 +489,6 @@ test.describe('ResultsParser', () => { }); }); - test('parses multiple failures successfully', async ({ testData }) => { - const resultsParser = new ResultsParser({ separateFlakyTests: false }); - resultsParser.addTestResult( - testData.suites[0].suites[0].title, - testData.suites[0].suites[0].tests[0], - [], - ); - resultsParser.addTestResult( - testData.suites[0].suites[0].title, - testData.suites[1].suites[0].tests[0], - [], - ); - const results = await resultsParser.getParsedResults(); - expect(results.failures).toEqual([ - { - test: 'basic test failure 2', - failureReason: - 'expect(received).toHaveText(expected)\n\nExpected string: "Playwright Fail"\nReceived string: "Playwright"\nCall log:\n - expect.toHaveText with timeout 1000ms\n - waiting for selector ".navbar__inner .navbar__title"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n\r\nError: expect(received).toHaveText(expected)\n\nExpected string: "Playwright Fail"\nReceived string: "Playwright"\nCall log:\n - expect.toHaveText with timeout 1000ms\n - waiting for selector ".navbar__inner .navbar__title"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n - selector resolved to Playwright\n - unexpected value "Playwright"\n\n at /home/ry/_repo/playwright-slack-report/tests/t1.spec.ts:17:23\r\n', - }, - ]); - }); - - test('separates flaky tests from successful tests when flaky option is enabled', async ({ - testData, - }) => { - const resultsParser = new ResultsParser({ separateFlakyTests: true }); - - // add the flaky test run 1 - failed - resultsParser.addTestResult( - testData.suites[0].suites[0].title, - testData.suites[2].suites[0].tests[0], - [], - ); - - // add the flaky test run 2 - passed - resultsParser.addTestResult( - testData.suites[0].suites[0].title, - testData.suites[2].suites[0].tests[1], - [], - ); - - // add a non-flaky passed test - resultsParser.addTestResult( - testData.suites[0].suites[0].title, - testData.suites[2].suites[0].tests[2], - [], - ); - - const results = await resultsParser.getParsedResults(); - expect(results.passed).toBe(1); - expect(results.failures.length).toBe(0); - expect(results.flaky).toBe(1); - }); - test('getTestName(...) generates correct test name', async ({}) => { expect(ResultsParser.getTestName({ name: 'Login' })).toEqual('Login'); expect(