From 42543a48a7eca6c11430a7b4cf610130747cf5e4 Mon Sep 17 00:00:00 2001
From: Marcin Strzyz <37447884+mastrzyz@users.noreply.github.com>
Date: Thu, 17 Aug 2023 00:48:59 -0700
Subject: [PATCH] fix: fixed `PW_TEST_HTML_REPORT_OPEN`  + more type safe + doc
 (#24571)

Co-authored-by: Max Schmitt <max@schmitt.mx>
---
 docs/src/test-reporters-js.md                 |  2 +-
 .../playwright-test/src/reporters/html.ts     | 23 +++++++++++++++++--
 tests/playwright-test/reporter-html.spec.ts   | 17 ++++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/docs/src/test-reporters-js.md b/docs/src/test-reporters-js.md
index 121bb2b454..4641386ec7 100644
--- a/docs/src/test-reporters-js.md
+++ b/docs/src/test-reporters-js.md
@@ -157,7 +157,7 @@ npx playwright test --reporter=html
 ```
 
 By default, HTML report is opened automatically if some of the tests failed. You can control this behavior via the
-`open` property in the Playwright config. The possible values for that property are `always`, `never` and `on-failure`
+`open` property in the Playwright config or the `PW_TEST_HTML_REPORT_OPEN` environmental variable. The possible values for that property are `always`, `never` and `on-failure`
 (default).
 
 You can also configure `host` and `port` that are used to serve the HTML report.
diff --git a/packages/playwright-test/src/reporters/html.ts b/packages/playwright-test/src/reporters/html.ts
index e7c2d4da25..ba78c372f9 100644
--- a/packages/playwright-test/src/reporters/html.ts
+++ b/packages/playwright-test/src/reporters/html.ts
@@ -40,7 +40,14 @@ type TestEntry = {
   testCaseSummary: TestCaseSummary
 };
 
-type HtmlReportOpenOption = 'always' | 'never' | 'on-failure';
+
+const htmlReportOptions = ['always', 'never', 'on-failure'];
+type HtmlReportOpenOption = (typeof htmlReportOptions)[number];
+
+const isHtmlReportOption = (type: string): type is HtmlReportOpenOption => {
+  return htmlReportOptions.includes(type);
+};
+
 type HtmlReporterOptions = {
   configDir: string,
   outputFolder?: string,
@@ -100,7 +107,7 @@ class HtmlReporter extends EmptyReporter {
     const outputFolder = reportFolderFromEnv() ?? resolveReporterOutputPath('playwright-report', this._options.configDir, this._options.outputFolder);
     return {
       outputFolder,
-      open: process.env.PW_TEST_HTML_REPORT_OPEN as any || this._options.open || 'on-failure',
+      open: getHtmlReportOptionProcessEnv() || this._options.open || 'on-failure',
       attachmentsBaseURL: this._options.attachmentsBaseURL || 'data/'
     };
   }
@@ -137,6 +144,18 @@ function reportFolderFromEnv(): string | undefined {
   return undefined;
 }
 
+function getHtmlReportOptionProcessEnv(): HtmlReportOpenOption | undefined {
+  const processKey = 'PW_TEST_HTML_REPORT_OPEN';
+  const htmlOpenEnv = process.env[processKey];
+  if (!htmlOpenEnv)
+    return undefined;
+  if (!isHtmlReportOption(htmlOpenEnv)) {
+    console.log(colors.red(`Configuration Error: HTML reporter Invalid value for ${processKey}: ${htmlOpenEnv}. Valid values are: ${htmlReportOptions.join(', ')}`));
+    return undefined;
+  }
+  return htmlOpenEnv;
+}
+
 function standaloneDefaultFolder(): string {
   return reportFolderFromEnv() ?? resolveReporterOutputPath('playwright-report', process.cwd(), undefined);
 }
diff --git a/tests/playwright-test/reporter-html.spec.ts b/tests/playwright-test/reporter-html.spec.ts
index dca8a20a13..dfc82e8af3 100644
--- a/tests/playwright-test/reporter-html.spec.ts
+++ b/tests/playwright-test/reporter-html.spec.ts
@@ -86,6 +86,23 @@ for (const useIntermediateMergeReport of [false, true] as const) {
     });
 
 
+    test('should not throw when PW_TEST_HTML_REPORT_OPEN value is invalid', async ({ runInlineTest, page, showReport }, testInfo) => {
+      const invalidOption = 'invalid-option';
+      const result = await runInlineTest({
+        'playwright.config.ts': `
+          module.exports = { preserveOutput: 'failures-only' };
+        `,
+        'a.test.js': `
+          import { test, expect } from '@playwright/test';
+          test('passes', async ({ page }, testInfo) => {
+            expect(2).toEqual(2);
+          });
+        `,
+      }, { reporter: 'dot,html' }, { PW_TEST_HTML_REPORT_OPEN: invalidOption });
+      expect(result.exitCode).toBe(0);
+      expect(result.passed).toBe(1);
+    });
+
     test('should not throw when attachment is missing', async ({ runInlineTest, page, showReport }, testInfo) => {
       const result = await runInlineTest({
         'playwright.config.ts': `