Skip writing snapshots when tests fail

Author: SteveSandersonMS

dotnet/test/Harness/CapiProxy.cs

@@ -84,15 +84,16 @@ async Task<string> StartCoreAsync()
         }
     }
 
-    public async Task StopAsync()
+    public async Task StopAsync(bool skipWritingCache = false)
     {
         if (_startupTask != null)
         {
             try
             {
                 var url = await _startupTask;
+                var stopUrl = skipWritingCache ? $"{url}/stop?skipWritingCache=true" : $"{url}/stop";
                 using var client = new HttpClient();
-                await client.PostAsync($"{url}/stop", null);
+                await client.PostAsync(stopUrl, null);
             }
             catch { /* Best effort */ }
         }

dotnet/test/Harness/E2ETestContext.cs

@@ -101,7 +101,9 @@ public IReadOnlyDictionary<string, string> GetEnvironment()
 
     public async ValueTask DisposeAsync()
     {
-        await _proxy.DisposeAsync();
+        // Skip writing snapshots in CI to avoid corrupting them on test failures
+        var isCI = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("CI"));
+        await _proxy.StopAsync(skipWritingCache: isCI);
 
         try { if (Directory.Exists(HomeDir)) Directory.Delete(HomeDir, true); } catch { }
         try { if (Directory.Exists(WorkDir)) Directory.Delete(WorkDir, true); } catch { }

go/e2e/testharness/context.go

@@ -42,7 +42,8 @@ type TestContext struct {
 	WorkDir  string
 	ProxyURL string
 
-	proxy *CapiProxy
+	proxy      *CapiProxy
+	testFailed bool
 }
 
 // NewTestContext creates a new test context with isolated directories and a replaying proxy.
@@ -82,7 +83,7 @@ func NewTestContext(t *testing.T) *TestContext {
 	}
 
 	t.Cleanup(func() {
-		ctx.Close()
+		ctx.Close(t.Failed())
 	})
 
 	return ctx
@@ -113,9 +114,9 @@ func (c *TestContext) ConfigureForTest(t *testing.T) {
 }
 
 // Close cleans up the test context resources.
-func (c *TestContext) Close() {
+func (c *TestContext) Close(testFailed bool) {
 	if c.proxy != nil {
-		c.proxy.Stop()
+		c.proxy.StopWithOptions(testFailed)
 	}
 	if c.HomeDir != "" {
 		os.RemoveAll(c.HomeDir)

go/e2e/testharness/proxy.go

@@ -75,6 +75,12 @@ func (p *CapiProxy) Start() (string, error) {
 
 // Stop gracefully shuts down the proxy server.
 func (p *CapiProxy) Stop() error {
+	return p.StopWithOptions(false)
+}
+
+// StopWithOptions gracefully shuts down the proxy server.
+// If skipWritingCache is true, the proxy won't write captured exchanges to disk.
+func (p *CapiProxy) StopWithOptions(skipWritingCache bool) error {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
@@ -84,8 +90,12 @@ func (p *CapiProxy) Stop() error {
 
 	// Send stop request to the server
 	if p.proxyURL != "" {
+		stopURL := p.proxyURL + "/stop"
+		if skipWritingCache {
+			stopURL += "?skipWritingCache=true"
+		}
 		// Best effort - ignore errors
-		resp, err := http.Post(p.proxyURL+"/stop", "application/json", nil)
+		resp, err := http.Post(stopURL, "application/json", nil)
 		if err == nil {
 			resp.Body.Close()
 		}

nodejs/test/e2e/harness/CapiProxy.ts

@@ -43,8 +43,11 @@ export class CapiProxy {
         return await response.json();
     }
 
-    async stop(): Promise<void> {
-        const response = await fetch(`${this.proxyUrl}/stop`, { method: "POST" });
+    async stop(skipWritingCache?: boolean): Promise<void> {
+        const url = skipWritingCache
+            ? `${this.proxyUrl}/stop?skipWritingCache=true`
+            : `${this.proxyUrl}/stop`;
+        const response = await fetch(url, { method: "POST" });
         expect(response.ok).toBe(true);
     }
 }

nodejs/test/e2e/harness/sdkTestContext.ts

@@ -8,7 +8,7 @@ import os from "os";
 import { basename, dirname, join, resolve } from "path";
 import { rimraf } from "rimraf";
 import { fileURLToPath } from "url";
-import { afterAll, afterEach, beforeEach, TestContext } from "vitest";
+import { afterAll, afterEach, beforeEach, onTestFailed, TestContext } from "vitest";
 import { CopilotClient } from "../../../src";
 import { CapiProxy } from "./CapiProxy";
 import { retry } from "./sdkTestHelper";
@@ -44,8 +44,15 @@ export async function createSdkTestContext() {
 
     const harness = { homeDir, workDir, openAiEndpoint, copilotClient, env };
 
+    // Track if any test fails to avoid writing corrupted snapshots
+    let anyTestFailed = false;
+
     // Wire up to Vitest lifecycle
     beforeEach(async (testContext) => {
+        onTestFailed(() => {
+            anyTestFailed = true;
+        });
+
         await openAiEndpoint.updateConfig({
             filePath: getTrafficCapturePath(testContext),
             workDir,
@@ -63,7 +70,7 @@ export async function createSdkTestContext() {
 
     afterAll(async () => {
         await copilotClient.stop();
-        await openAiEndpoint.stop();
+        await openAiEndpoint.stop(anyTestFailed);
         await rmDir("remove e2e test homeDir", homeDir);
         await rmDir("remove e2e test workDir", workDir);
     });

python/e2e/conftest.py

@@ -1,17 +1,34 @@
 """Shared pytest fixtures for e2e tests."""
 
+import pytest
 import pytest_asyncio
 
 from .testharness import E2ETestContext
 
 
+# Track if any test failed to avoid writing corrupted snapshots
+_any_test_failed = False
+
+
+@pytest.hookimpl(tryfirst=True, hookwrapper=True)
+def pytest_runtest_makereport(item, call):
+    """Track test failures to avoid writing corrupted snapshots."""
+    global _any_test_failed
+    outcome = yield
+    rep = outcome.get_result()
+    if rep.when == "call" and rep.failed:
+        _any_test_failed = True
+
+
 @pytest_asyncio.fixture(scope="module", loop_scope="module")
 async def ctx():
     """Create and teardown a test context shared across all tests in this module."""
+    global _any_test_failed
+    _any_test_failed = False  # Reset for each module
     context = E2ETestContext()
     await context.setup()
     yield context
-    await context.teardown()
+    await context.teardown(test_failed=_any_test_failed)
 
 
 @pytest_asyncio.fixture(autouse=True, loop_scope="module")

python/e2e/testharness/context.py

@@ -73,14 +73,18 @@ async def setup(self):
             }
         )
 
-    async def teardown(self):
-        """Clean up the test context."""
+    async def teardown(self, test_failed: bool = False):
+        """Clean up the test context.
+        
+        Args:
+            test_failed: If True, skip writing snapshots to avoid corruption.
+        """
         if self._client:
             await self._client.stop()
             self._client = None
 
         if self._proxy:
-            await self._proxy.stop()
+            await self._proxy.stop(skip_writing_cache=test_failed)
             self._proxy = None
 
         if self.home_dir and os.path.exists(self.home_dir):

python/e2e/testharness/proxy.py

@@ -59,16 +59,23 @@ async def start(self) -> str:
         self._proxy_url = match.group(1)
         return self._proxy_url
 
-    async def stop(self):
-        """Gracefully shut down the proxy server."""
+    async def stop(self, skip_writing_cache: bool = False):
+        """Gracefully shut down the proxy server.
+        
+        Args:
+            skip_writing_cache: If True, the proxy won't write captured exchanges to disk.
+        """
         if not self._process:
             return
 
         # Send stop request to the server
         if self._proxy_url:
             try:
+                stop_url = f"{self._proxy_url}/stop"
+                if skip_writing_cache:
+                    stop_url += "?skipWritingCache=true"
                 async with httpx.AsyncClient() as client:
-                    await client.post(f"{self._proxy_url}/stop")
+                    await client.post(stop_url)
             except Exception:
                 pass  # Best effort
 

test/harness/replayingCapiProxy.ts

@@ -146,12 +146,13 @@ export class ReplayingCapiProxy extends CapturingHttpProxy {
 
         // Handle /stop endpoint for stopping the proxy
         if (
-          options.requestOptions.path === "/stop" &&
+          options.requestOptions.path?.startsWith("/stop") &&
           options.requestOptions.method === "POST"
         ) {
+          const skipWritingCache = options.requestOptions.path.includes("skipWritingCache=true");
           options.onResponseStart(200, {});
           options.onResponseEnd();
-          await this.stop();
+          await this.stop(skipWritingCache);
           process.exit(0);
         }