ref(e2e-tests): Make startCommand optional in shared playwright config (#12842)

Not all our e2e tests require a `startCommand` as we sometimes directly
invoke an application from within tests. For example in our AWS lambda
e2e tests. This PR makes the `startCommand` option optional so that we
can avoid a larger override of the entire `webServer` playwright config
object.

Author: Lms24

dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/playwright.config.ts

@@ -1,79 +1,3 @@
-import type { PlaywrightTestConfig } from '@playwright/test';
-import { devices } from '@playwright/test';
+import { getPlaywrightConfig } from '@sentry-internal/test-utils';
 
-// Fix urls not resolving to localhost on Node v17+
-// See: https://github.com/axios/axios/issues/3821#issuecomment-1413727575
-import { setDefaultResultOrder } from 'dns';
-setDefaultResultOrder('ipv4first');
-
-const eventProxyPort = 3031;
-const lambdaPort = 3030;
-
-/**
- * See https://playwright.dev/docs/test-configuration.
- */
-const config: PlaywrightTestConfig = {
-  testDir: './tests',
-  /* Maximum time one test can run for. */
-  timeout: 150_000,
-  expect: {
-    /**
-     * Maximum time expect() should wait for the condition to be met.
-     * For example in `await expect(locator).toHaveText();`
-     */
-    timeout: 5000,
-  },
-  /* Run tests in files in parallel */
-  fullyParallel: true,
-  /* Fail the build on CI if you accidentally left test.only in the source code. */
-  forbidOnly: !!process.env.CI,
-  /* Retry on CI only */
-  retries: 0,
-  /* Reporter to use. See https://playwright.dev/docs/test-reporters */
-  reporter: 'list',
-  /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
-  use: {
-    /* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
-    actionTimeout: 0,
-
-    /* Base URL to use in actions like `await page.goto('/')`. */
-    baseURL: `http://localhost:${lambdaPort}`,
-
-    /* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
-    trace: 'on-first-retry',
-  },
-
-  /* Configure projects for major browsers */
-  projects: [
-    {
-      name: 'chromium',
-      use: {
-        ...devices['Desktop Chrome'],
-      },
-    },
-    // For now we only test Chrome!
-    // {
-    //   name: 'firefox',
-    //   use: {
-    //     ...devices['Desktop Firefox'],
-    //   },
-    // },
-    // {
-    //   name: 'webkit',
-    //   use: {
-    //     ...devices['Desktop Safari'],
-    //   },
-    // },
-  ],
-
-  /* Run your local dev server before starting the tests */
-  webServer: [
-    {
-      command: `node start-event-proxy.mjs && pnpm wait-port ${eventProxyPort}`,
-      port: eventProxyPort,
-      stdout: 'pipe',
-    },
-  ],
-};
-
-export default config;
+export default getPlaywrightConfig();

dev-packages/e2e-tests/test-applications/aws-serverless-esm/playwright.config.ts

@@ -1,27 +1,3 @@
 import { getPlaywrightConfig } from '@sentry-internal/test-utils';
 
-// Fix urls not resolving to localhost on Node v17+
-// See: https://github.com/axios/axios/issues/3821#issuecomment-1413727575
-import { setDefaultResultOrder } from 'dns';
-setDefaultResultOrder('ipv4first');
-
-const eventProxyPort = 3031;
-
-/**
- * See https://playwright.dev/docs/test-configuration.
- */
-const config = getPlaywrightConfig(
-  { startCommand: '' },
-  {
-    /* Run your local dev server before starting the tests */
-    webServer: [
-      {
-        command: `node start-event-proxy.mjs && pnpm wait-port ${eventProxyPort}`,
-        port: eventProxyPort,
-        stdout: 'pipe',
-      },
-    ],
-  },
-);
-
-export default config;
+export default getPlaywrightConfig();

dev-packages/test-utils/.eslintrc.js

@@ -3,5 +3,12 @@ module.exports = {
     node: true,
   },
   extends: ['../../.eslintrc.js'],
-  overrides: [],
+  overrides: [
+    {
+      files: ['**/*.ts'],
+      rules: {
+        '@sentry-internal/sdk/no-optional-chaining': 'off',
+      },
+    },
+  ],
 };

dev-packages/test-utils/src/playwright-config.ts

@@ -2,19 +2,19 @@ import type { PlaywrightTestConfig } from '@playwright/test';
 
 /** Get a playwright config to use in an E2E test app. */
 export function getPlaywrightConfig(
-  options: {
-    startCommand: string;
+  options?: {
+    startCommand?: string;
     port?: number;
     eventProxyPort?: number;
     eventProxyFile?: string;
   },
   overwriteConfig?: Partial<PlaywrightTestConfig>,
 ): PlaywrightTestConfig {
   const testEnv = process.env['TEST_ENV'] || 'production';
-  const appPort = options.port || 3030;
-  const eventProxyPort = options.eventProxyPort || 3031;
-  const eventProxyFile = options.eventProxyFile || 'start-event-proxy.mjs';
-  const { startCommand } = options;
+  const appPort = options?.port || 3030;
+  const eventProxyPort = options?.eventProxyPort || 3031;
+  const eventProxyFile = options?.eventProxyFile || 'start-event-proxy.mjs';
+  const startCommand = options?.startCommand;
 
   /**
    * See https://playwright.dev/docs/test-configuration.
@@ -76,18 +76,23 @@ export function getPlaywrightConfig(
         stdout: 'pipe',
         stderr: 'pipe',
       },
-      {
-        command: startCommand,
-        port: appPort,
-        stdout: 'pipe',
-        stderr: 'pipe',
-        env: {
-          PORT: appPort.toString(),
-        },
-      },
     ],
   };
 
+  if (startCommand) {
+    // @ts-expect-error - we set `config.webserver` to an array above.
+    // TS just can't infer that and thinks it could also be undefined or an object.
+    config.webServer.push({
+      command: startCommand,
+      port: appPort,
+      stdout: 'pipe',
+      stderr: 'pipe',
+      env: {
+        PORT: appPort.toString(),
+      },
+    });
+  }
+
   return {
     ...config,
     ...overwriteConfig,