From fe433b6005ac3e544501ab9d6c481864c84b20f2 Mon Sep 17 00:00:00 2001
From: Ryan Waskiewicz <ryanwaskiewicz@gmail.com>
Date: Tue, 30 May 2023 07:47:02 -0700
Subject: [PATCH] fix(e2e): honor devtools and browserDevtools settings (#4403)

this commit fixes a bug where configuration options passed to puppeteer
would not be forwarded properly. specifically, setting the `--devtools`
flag on the command line and setting `browserDevtools` in a project's
configuration would be ignored.

the setting of these fields has been moved to the testing configuration
validation step of the test task - this allows us to make the correct
decision regarding setting the `browserDevtools` flag based on:
- whether `CI` is enabled (prefer headless mode in this scenario)
- `--devtools` or `browserHeadless` is set (used when `CI` is not
  enabled, disable headless mode)
---
 .../config/test/validate-testing.spec.ts      | 33 +++++++++++++++++++
 src/compiler/config/validate-testing.ts       |  3 ++
 src/testing/puppeteer/puppeteer-browser.ts    |  2 --
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/compiler/config/test/validate-testing.spec.ts b/src/compiler/config/test/validate-testing.spec.ts
index fa368aeb332..2a7bbe08b36 100644
--- a/src/compiler/config/test/validate-testing.spec.ts
+++ b/src/compiler/config/test/validate-testing.spec.ts
@@ -82,6 +82,39 @@ describe('validateTesting', () => {
     });
   });
 
+  describe('devTools', () => {
+    it('ignores devTools settings if CI is enabled', () => {
+      userConfig.flags = { ...flags, ci: true, devtools: true, e2e: true };
+      userConfig.testing = {};
+
+      const { config } = validateConfig(userConfig, mockLoadConfigInit());
+
+      expect(config.testing.browserDevtools).toBeUndefined();
+    });
+
+    it('sets browserDevTools to true when the devtools flag is set', () => {
+      userConfig.flags = { ...flags, devtools: true, e2e: true };
+      userConfig.testing = {};
+
+      const { config } = validateConfig(userConfig, mockLoadConfigInit());
+
+      expect(config.testing.browserDevtools).toBe(true);
+      // browserHeadless must be false to enabled dev tools (which are headful by definition)
+      expect(config.testing.browserHeadless).toBe(false);
+    });
+
+    it("sets browserDevTools to true when set in a project's config", () => {
+      userConfig.flags = { ...flags, devtools: false, e2e: true };
+      userConfig.testing = { browserDevtools: true };
+
+      const { config } = validateConfig(userConfig, mockLoadConfigInit());
+
+      expect(config.testing.browserDevtools).toBe(true);
+      // browserHeadless must be false to enabled dev tools (which are headful by definition)
+      expect(config.testing.browserHeadless).toBe(false);
+    });
+  });
+
   describe('browserWaitUntil', () => {
     it('sets the default to "load" if no value is provided', () => {
       userConfig.flags = { ...flags, e2e: true };
diff --git a/src/compiler/config/validate-testing.ts b/src/compiler/config/validate-testing.ts
index a6e77c687b4..d6dd0ae43d1 100644
--- a/src/compiler/config/validate-testing.ts
+++ b/src/compiler/config/validate-testing.ts
@@ -39,6 +39,9 @@ export const validateTesting = (config: d.ValidatedConfig, diagnostics: d.Diagno
     addTestingConfigOption(testing.browserArgs, '--disable-setuid-sandbox');
     addTestingConfigOption(testing.browserArgs, '--disable-dev-shm-usage');
     testing.browserHeadless = testing.browserHeadless === 'new' ? 'new' : true;
+  } else if (config.flags.devtools || testing.browserDevtools) {
+    testing.browserDevtools = true;
+    testing.browserHeadless = false;
   }
 
   if (typeof testing.rootDir === 'string') {
diff --git a/src/testing/puppeteer/puppeteer-browser.ts b/src/testing/puppeteer/puppeteer-browser.ts
index e9e6d1534dc..b7d82bd4601 100644
--- a/src/testing/puppeteer/puppeteer-browser.ts
+++ b/src/testing/puppeteer/puppeteer-browser.ts
@@ -29,8 +29,6 @@ export async function startPuppeteerBrowser(config: ValidatedConfig) {
   env.__STENCIL_BROWSER_WAIT_UNTIL = config.testing.browserWaitUntil;
 
   if (config.flags.devtools) {
-    config.testing.browserDevtools = true;
-    config.testing.browserHeadless = false;
     env.__STENCIL_E2E_DEVTOOLS__ = 'true';
   }