Skip to content

Commit

Permalink
fix(reporters): improve detection of output folder clashes (#30607)
Browse files Browse the repository at this point in the history
When comparing `outputDir` and html-reporter `outputFolder`, we now make
sure that both paths end with a forward-slash.

Fixes #28677

---------

Co-authored-by: Georg Unterholzner <georg.unterholzner@dynatrace.com>
  • Loading branch information
georg-un and Georg Unterholzner authored May 2, 2024
1 parent c1fbc75 commit 8173cdc
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
7 changes: 6 additions & 1 deletion packages/playwright/src/reporters/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class HtmlReporter extends EmptyReporter {
this._attachmentsBaseURL = attachmentsBaseURL;
const reportedWarnings = new Set<string>();
for (const project of this.config.projects) {
if (outputFolder.startsWith(project.outputDir) || project.outputDir.startsWith(outputFolder)) {
if (this._isSubdirectory(outputFolder, project.outputDir) || this._isSubdirectory(project.outputDir, outputFolder)) {
const key = outputFolder + '|' + project.outputDir;
if (reportedWarnings.has(key))
continue;
Expand All @@ -113,6 +113,11 @@ class HtmlReporter extends EmptyReporter {
};
}

_isSubdirectory(parentDir: string, dir: string): boolean {
const relativePath = path.relative(parentDir, dir);
return !!relativePath && !relativePath.startsWith('..') && !path.isAbsolute(relativePath);
}

override onError(error: TestError): void {
this._topLevelErrors.push(error);
}
Expand Down
20 changes: 20 additions & 0 deletions tests/playwright-test/reporter-html.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1094,6 +1094,26 @@ for (const useIntermediateMergeReport of [false] as const) {
expect(output).toContain('html-report');
});

test('it should only identify exact matches as clashing folders', async ({ runInlineTest, useIntermediateMergeReport }) => {
test.skip(useIntermediateMergeReport);
const result = await runInlineTest({
'playwright.config.ts': `
module.exports = {
reporter: [['html', { outputFolder: 'test-results-html' }]]
}
`,
'a.test.js': `
import { test, expect } from '@playwright/test';
test('passes', async ({}) => {
});
`,
});
expect(result.exitCode).toBe(0);
const output = result.output;
expect(output).not.toContain('Configuration Error');
expect(output).toContain('test-results-html');
});

test.describe('report location', () => {
test('with config should create report relative to config', async ({ runInlineTest, useIntermediateMergeReport }, testInfo) => {
test.skip(useIntermediateMergeReport);
Expand Down

0 comments on commit 8173cdc

Please sign in to comment.