Skip to content

Commit

Permalink
Fix incorrect stats calculations (#68)
Browse files Browse the repository at this point in the history
* WIP: fix stats

* remove separate flakey config

* shuffle expected stats

* add config for nyc

* update node version gh wf to 18.18.2
  • Loading branch information
ryanrosello-og authored Nov 11, 2023
1 parent 31238b2 commit c7b2fc2
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 151 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,4 @@ dist/
# VS Code files
.vscode
bot
playwright-report/
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 11 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "[email protected]:ryanrosello-og/playwright-slack-report.git",
Expand All @@ -44,5 +44,14 @@
"report",
"playwright",
"typescript"
]
],
"nyc": {
"all": true,
"include": [
"src/**/*.ts"
],
"exclude": [
"./src/custom_block/my_block.ts"
]
}
}
3 changes: 3 additions & 0 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ require('dotenv').config();

const config: PlaywrightTestConfig = {
forbidOnly: !!process.env.CI,
workers: 4,
retries: 1,
use: {
trace: 'on',
screenshot: 'on',
Expand Down Expand Up @@ -45,6 +47,7 @@ const config: PlaywrightTestConfig = {
},
],
['dot'],
['html'],
],
};
export default config;
84 changes: 15 additions & 69 deletions src/ResultsParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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<SummaryResults> {
async getParsedResults(allTests: Array<TestCase>): Promise<SummaryResults> {
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;
}
Expand All @@ -95,35 +86,6 @@ export default class ResultsParser {
return failures;
}

async getFlakes(): Promise<Array<flaky>> {
const flaky: Array<flaky> = [];
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<Array<pass>> {
const passes: Array<pass> = [];
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) {
Expand Down Expand Up @@ -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<pass>, flakes: Array<flaky>) {
const _passes: Map<string, pass> = new Map();

for (const pass of passes) {
_passes.set(pass.test, pass);
}

for (const flake of flakes) {
_passes.delete(flake.test);
}

return [..._passes.values()];
}
}
9 changes: 2 additions & 7 deletions src/SlackReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ class SlackReporter implements Reporter {

private suite!: Suite;

private separateFlaky: boolean = false;

logs: string[] = [];

onBegin(fullConfig: FullConfig, suite: Suite): void {
Expand Down Expand Up @@ -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
Expand All @@ -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 (
Expand Down
9 changes: 0 additions & 9 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,3 @@ export type failure = {
test: string;
failureReason: string;
};

export type flaky = {
test: string;
retry: number;
};

export type pass = {
test: string;
};
88 changes: 28 additions & 60 deletions tests/ResultsParser.spec.ts
Original file line number Diff line number Diff line change
@@ -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 }>({
Expand Down Expand Up @@ -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],
Expand All @@ -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: [
Expand Down Expand Up @@ -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 <b class="navbar__title text--truncate">Playwright</b>\n - unexpected value "Playwright"\n - selector resolved to <b class="navbar__title text--truncate">Playwright</b>\n - unexpected value "Playwright"\n - selector resolved to <b class="navbar__title text--truncate">Playwright</b>\n - unexpected value "Playwright"\n - selector resolved to <b class="navbar__title text--truncate">Playwright</b>\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 <b class="navbar__title text--truncate">Playwright</b>\n - unexpected value "Playwright"\n - selector resolved to <b class="navbar__title text--truncate">Playwright</b>\n - unexpected value "Playwright"\n - selector resolved to <b class="navbar__title text--truncate">Playwright</b>\n - unexpected value "Playwright"\n - selector resolved to <b class="navbar__title text--truncate">Playwright</b>\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(
Expand Down

0 comments on commit c7b2fc2

Please sign in to comment.