fix: use retain-on-failure in CI, remove manual tracing

Address review comments:
- Use 'retain-on-failure' in CI to limit artifact size, 'on' locally
- Remove manual tracing.start/stop from fixture — Playwright's built-in
  use.trace config handles it automatically, avoiding 'Tracing has been
  already started' conflicts

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: wenytang-ms

test/e2e/fixtures/baseTest.ts

@@ -136,23 +136,15 @@ export const test = base.extend<TestFixtures>({
         await page.waitForTimeout(3_000);
         await dismissAllNotifications(page);
 
-        // 4. Start tracing — captures screenshots and DOM snapshots at every
-        //    Playwright action so failures can be replayed step-by-step.
-        await page.context().tracing.start({ screenshots: true, snapshots: true, title: testInfo.title });
+        // Tracing is handled by Playwright's built-in `use.trace` config
+        // (see playwright.config.ts). No manual tracing.start/stop needed.
 
         // ---- hand off to the test ----
         await use(page);
 
         // ---- teardown ----
-        // Always save trace — on failure it's essential for debugging,
-        // on success it's useful for verifying the interaction flow.
-        const tracePath = testInfo.outputPath("trace.zip");
-        try {
-            await page.context().tracing.stop({ path: tracePath });
-            testInfo.attachments.push({ name: "trace", path: tracePath, contentType: "application/zip" });
-        } catch {
-            // Tracing may not have been started
-        }
+        // Trace saving is handled automatically by Playwright's `use.trace`
+        // config — no manual tracing.stop() needed here.
 
         await electronApp.close();
 

test/e2e/playwright.config.ts

@@ -20,12 +20,10 @@ export default defineConfig({
     },
     globalSetup: path.join(__dirname, "globalSetup.ts"),
     use: {
-        // Automatically take a screenshot when a test fails.
-        screenshot: "only-on-failure",
-        // Capture full trace on every test run (includes screenshots, DOM
-        // snapshots, and network at each Playwright action). This makes it
-        // easy to diagnose failures — especially when reviewed by AI.
-        trace: "on",
+        // Capture full trace on every test run locally (includes screenshots,
+        // DOM snapshots, and network at each Playwright action). In CI,
+        // retain traces only for failing tests to limit artifact size.
+        trace: process.env.CI ? "retain-on-failure" : "on",
     },
     outputDir: path.join(__dirname, "..", "..", "test-results", "e2e"),
 });