don't actually need the env var

Author: david-crespo

package.json

@@ -22,7 +22,7 @@
     "update-snapshots": "npm test run -- -u",
     "e2e": "playwright test",
     "e2ec": "playwright test --project=chrome",
-    "e2e-coverage": "rm -rf .nyc_output coverage && COVERAGE=1 npm run e2ec && nyc report --reporter=html --report-dir=coverage && echo 'report at coverage/index.html'",
+    "e2e-coverage": "rm -rf .nyc_output coverage && npm run e2ec && nyc report --reporter=html --report-dir=coverage && echo 'report at coverage/index.html'",
     "visual:baseline": "./tools/generate-visual-baseline.sh",
     "visual:compare": "playwright test -c playwright.visual.config.ts",
     "lint": "oxlint --type-aware",

playwright.config.ts

@@ -50,7 +50,7 @@ export default {
   ],
   // use different port so it doesn't conflict with local dev server
   webServer: {
-    command: `${process.env.COVERAGE ? 'VITE_COVERAGE=true ' : ''}npm run start:msw -- --port 4009`,
+    command: 'npm run start:msw -- --port 4009',
     port: 4009,
   },
 } satisfies PlaywrightTestConfig

test/e2e/fixtures.ts

@@ -9,20 +9,13 @@ import { mkdirSync, writeFileSync } from 'fs'
 import { resolve } from 'path'
 import { test as base, type Page } from '@playwright/test'
 
-const coverageEnabled = !!process.env.COVERAGE
 const coverageDir = resolve('.nyc_output')
 
 // Counter for unique filenames within this worker process
 let coverageCounter = 0
 
 async function collectCoverage(page: Page): Promise<void> {
-  if (!coverageEnabled) return
-
-  const coverage = await page.evaluate(() => {
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    return (window as any).__coverage__
-  })
-
+  const coverage = await page.evaluate(() => (window as any).__coverage__) // eslint-disable-line @typescript-eslint/no-explicit-any
   if (!coverage) return
 
   mkdirSync(coverageDir, { recursive: true })

vite.config.ts

@@ -135,18 +135,28 @@ export default defineConfig(({ mode }) => ({
     }),
     react(),
     apiMode === 'remote' && basicSsl(),
-    !!process.env.VITE_COVERAGE &&
-      istanbul({
-        include: 'app/**/*',
-        exclude: [
-          'app/api/__generated__',
-          '**/*.spec.*',
-          'app/api/window.ts',
-          'app/components/MswBanner.tsx',
-        ],
-        extension: ['.ts', '.tsx'],
-        requireEnv: true,
-      }),
+    // Docs: https://github.com/ifaxity/vite-plugin-istanbul
+    istanbul({
+      include: 'app/**/*',
+      exclude: [
+        'app/api/__generated__',
+        '**/*.spec.*',
+        'app/api/window.ts',
+        'app/components/MswBanner.tsx',
+      ],
+      extension: ['.ts', '.tsx'],
+      // We technically do not need checkProd to prevent instrumenting in
+      // production because the plugin won't instrument in build mode by default
+      // (tested by comparing with forceBuildInstrument: true, which increases
+      // the bundle size a ton), and we only use build mode in prod.
+      //
+      // Initially this used requireEnv to only instrument when VITE_COVERAGE
+      // is set, but instrumenting didn't seem to slow the tests down at all. So
+      // it seems fine to always instrument and always collect coverage if it's
+      // present (see test/e2e/fixtures.ts`). This keeps things simple in CI
+      // as well.
+      checkProd: true,
+    }),
   ],
   html: {
     // don't include a placeholder nonce in production.