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)

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 };

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') {

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';
   }