Address review feedback: async init, SkipConnectionCheck throw, restore helix/copilot env vars, delete scripts

Copilot · AndriySvyryd · Copilot · commit 6df36b8093d6 · 2026-03-25T19:44:25.000Z
- Make TestEnvironment initialization fully async (InitializeAsync) - Set DefaultConnection and HttpMessageHandler values during async init - If SkipConnectionCheck=true, let testcontainer failures throw - Create HttpClient instance outside the lambda in ApplyConfiguration - Inline IsTestContainer (removed property) - Restore copilot-setup-steps.yml env vars (EmulatorType, DefaultConnection, SkipConnectionCheck) - Restore helix.proj Windows.11 block and SkipConnectionCheck for Ubuntu - Delete eng/testing/run-cosmos-container.{sh,ps1} - Update SKILL.md Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/efcore/sessions/7f2ba766-d1b7-484b-899c-4b6d320a97e5
diff --git a/.agents/skills/cosmos-provider/SKILL.md b/.agents/skills/cosmos-provider/SKILL.md
@@ -25,12 +25,12 @@ Non-relational provider with its own parallel query pipeline. Uses JSON for docu
 
 ### Automatic Testcontainer Startup
 
-Cosmos functional tests automatically manage the emulator lifecycle via [Testcontainers](https://testcontainers.com/modules/cosmodb/?language=dotnet) (`Testcontainers.CosmosDb` NuGet package). The initialization logic in `TestEnvironment.cs` follows this order:
+Cosmos functional tests automatically manage the emulator lifecycle via [Testcontainers](https://testcontainers.com/modules/cosmodb/?language=dotnet) (`Testcontainers.CosmosDb` NuGet package). The async initialization logic in `TestEnvironment.InitializeAsync()` (called from `CosmosTestStore.IsConnectionAvailableAsync()`) follows this order:
 
 1. **Configured endpoint**: If `Test__Cosmos__DefaultConnection` env var (or `cosmosConfig.json` / `cosmosConfig.test.json`) is set, it is used directly — no container is started.
 2. **Local emulator probe**: A quick HTTPS probe is sent to `https://localhost:8081`. If a running emulator responds, it is used.
 3. **Testcontainer fallback**: If neither of the above succeeds, a `CosmosDbContainer` is started with the Linux emulator image (`mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview`). The container is disposed on process exit.
-4. **Graceful skip**: If Docker is unavailable and no emulator is reachable, the default endpoint is used and `IsConnectionAvailableAsync()` returns `false`, causing tests to be skipped.
+4. **Graceful skip**: If Docker is unavailable and no emulator is reachable, the default endpoint is used and `IsConnectionAvailableAsync()` returns `false`, causing tests to be skipped. However, if `Test__Cosmos__SkipConnectionCheck=true` is set (e.g. in CI), the testcontainer failure is **not** caught so that infrastructure problems surface immediately.
 
 ### Linux Emulator Detection
 
@@ -43,15 +43,11 @@ The Linux (vnext) emulator does **not** support transactional batches, so `Linux
 
 ### HttpClient Handling
 
-When a testcontainer is active, `CosmosDbContextOptionsBuilderExtensions.ApplyConfiguration` uses the container's `HttpMessageHandler` (a URI rewriter that routes requests to the mapped container port over HTTP). When connecting to a local HTTPS emulator, it uses `DangerousAcceptAnyServerCertificateValidator` instead.
-
-### Manual Scripts (Legacy)
-
-The shell scripts `eng/testing/run-cosmos-container.sh` and `eng/testing/run-cosmos-container.ps1` can still be used to manually start the emulator in Docker when needed, but they are no longer invoked by Helix or CI.
+When a testcontainer is active, `CosmosDbContextOptionsBuilderExtensions.ApplyConfiguration` uses the container's `HttpMessageHandler` (a URI rewriter that routes requests to the mapped container port over HTTP), captured once during initialization. When connecting to a local HTTPS emulator, it uses `DangerousAcceptAnyServerCertificateValidator` instead.
 
 ### Key Files
 
-- `test/EFCore.Cosmos.FunctionalTests/TestUtilities/TestEnvironment.cs` — connection auto-detection and testcontainer lifecycle
+- `test/EFCore.Cosmos.FunctionalTests/TestUtilities/TestEnvironment.cs` — async connection auto-detection and testcontainer lifecycle
 - `test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs` — test store creation, seeding, cleanup
 - `test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosDbContextOptionsBuilderExtensions.cs` — shared Cosmos options (execution strategy, timeout, HttpClient, Gateway mode)
 - `test/EFCore.Cosmos.FunctionalTests/TestUtilities/LinuxEmulatorSaveChangesInterceptor.cs` — disables transactional batches for the Linux emulator
diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml
@@ -63,5 +63,8 @@ jobs:
       - name: Export environment variables for the agent's session
         run: |
           echo "Test__SqlServer__DefaultConnection=Server=localhost;Database=test;User=SA;Password=PLACEHOLDERPass$$w0rd;Connect Timeout=60;ConnectRetryCount=0;Trust Server Certificate=true" >> "$GITHUB_ENV"
+          echo "Test__Cosmos__DefaultConnection=https://localhost:8081" >> "$GITHUB_ENV"
+          echo "Test__Cosmos__EmulatorType=linux" >> "$GITHUB_ENV"
+          echo "Test__Cosmos__SkipConnectionCheck=true" >> "$GITHUB_ENV"
           echo "DOTNET_ROOT=$PWD/.dotnet/" >> "$GITHUB_ENV"
           echo "$PWD/.dotnet/" >> $GITHUB_PATH
diff --git a/eng/helix.proj b/eng/helix.proj
@@ -65,6 +65,12 @@
     </XUnitProject>
   </ItemGroup>
 
+  <ItemGroup Condition = "'$(HelixTargetQueue.StartsWith(`Windows.11`))'">
+    <XUnitProject Update="$(CosmosTests)">
+      <PreCommands>$(PreCommands); set Test__Cosmos__SkipConnectionCheck=true</PreCommands>
+    </XUnitProject>
+  </ItemGroup>
+
   <!-- Start SqlServer instance for test projects which uses SqlServer on docker. Only run SqlServer tests. -->
   <ItemGroup Condition = "'$(HelixTargetQueue.Contains(`helix-sqlserver`))'">
     <XUnitProject Remove="$(RepoRoot)/test/**/*.csproj"/>
@@ -78,7 +84,9 @@
   <ItemGroup Condition = "'$(HelixTargetQueue)' == 'Ubuntu.2204.Amd64.XL.Open' OR '$(HelixTargetQueue)' == 'Ubuntu.2204.Amd64.XL'">
     <XUnitProject Remove="$(RepoRoot)/test/**/*.csproj"/>
     <XUnitProject Remove="$(RepoRoot)/test/**/*.fsproj"/>
-    <XUnitProject Include="$(CosmosTests)" />
+    <XUnitProject Include="$(CosmosTests)">
+      <PreCommands>$(PreCommands); export Test__Cosmos__SkipConnectionCheck=true</PreCommands>
+    </XUnitProject>
   </ItemGroup>
 
   <!-- Run tests that don't need SqlServer or Cosmos on bare Ubuntu -->
diff --git a/eng/testing/run-cosmos-container.ps1 b/eng/testing/run-cosmos-container.ps1
diff --git a/eng/testing/run-cosmos-container.sh b/eng/testing/run-cosmos-container.sh
diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosDbContextOptionsBuilderExtensions.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosDbContextOptionsBuilderExtensions.cs
@@ -9,18 +9,18 @@ public static class CosmosDbContextOptionsBuilderExtensions
 {
     public static CosmosDbContextOptionsBuilder ApplyConfiguration(this CosmosDbContextOptionsBuilder optionsBuilder)
     {
-        var handlerFactory = TestEnvironment.HttpMessageHandlerFactory;
+        var httpClient = TestEnvironment.HttpMessageHandler != null
+            ? new HttpClient(TestEnvironment.HttpMessageHandler)
+            : new HttpClient(
+                new HttpClientHandler
+                {
+                    ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator
+                });
 
         optionsBuilder
             .ExecutionStrategy(d => new TestCosmosExecutionStrategy(d))
             .RequestTimeout(TimeSpan.FromMinutes(20))
-            .HttpClientFactory(handlerFactory != null
-                ? () => new HttpClient(handlerFactory())
-                : () => new HttpClient(
-                    new HttpClientHandler
-                    {
-                        ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator
-                    }))
+            .HttpClientFactory(() => httpClient)
             .ConnectionMode(ConnectionMode.Gateway);
 
         return optionsBuilder;
diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/CosmosTestStore.cs
@@ -105,6 +105,8 @@ public override DbContextOptionsBuilder AddProviderOptions(DbContextOptionsBuild
 
     public static async ValueTask<bool> IsConnectionAvailableAsync()
     {
+        await TestEnvironment.InitializeAsync().ConfigureAwait(false);
+
         if (TestEnvironment.SkipConnectionCheck)
         {
             return true;
diff --git a/test/EFCore.Cosmos.FunctionalTests/TestUtilities/TestEnvironment.cs b/test/EFCore.Cosmos.FunctionalTests/TestUtilities/TestEnvironment.cs
@@ -23,71 +23,90 @@ public static class TestEnvironment
         .Build()
         .GetSection("Test:Cosmos");
 
-    private static readonly Lazy<(string Connection, CosmosDbContainer Container)> _connectionInfo = new(InitializeConnection);
+    private static CosmosDbContainer _container;
+    private static bool _initialized;
+    private static readonly SemaphoreSlim _initSemaphore = new(1, 1);
 
-    public static string DefaultConnection => _connectionInfo.Value.Connection;
+    public static string DefaultConnection { get; private set; }
 
-    private static CosmosDbContainer Container => _connectionInfo.Value.Container;
+    internal static HttpMessageHandler HttpMessageHandler { get; private set; }
 
-    public static bool IsTestContainer => Container != null;
-
-    internal static Func<HttpMessageHandler> HttpMessageHandlerFactory
-        => Container != null ? () => Container.HttpMessageHandler : null;
-
-    private static (string Connection, CosmosDbContainer Container) InitializeConnection()
+    public static async Task InitializeAsync()
     {
-        // If a connection string is specified (env var, config.json...), always use that.
-        var configured = Config["DefaultConnection"];
-        if (!string.IsNullOrEmpty(configured))
+        if (_initialized)
         {
-            return (configured, null);
+            return;
         }
 
-        // Try to connect to the default emulator endpoint.
-        if (TryProbeEmulator("https://localhost:8081"))
-        {
-            return ("https://localhost:8081", null);
-        }
+        await _initSemaphore.WaitAsync().ConfigureAwait(false);
 
-        // Try to start a testcontainer with the Linux emulator.
-        // Synchronous blocking is required here because this runs in a Lazy<T> initializer
-        // which cannot be async. This matches the pattern used in SQL Server's TestEnvironment.
         try
         {
-            var container = new CosmosDbBuilder("mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview")
-                .Build();
-            container.StartAsync().GetAwaiter().GetResult();
+            if (_initialized)
+            {
+                return;
+            }
 
-            AppDomain.CurrentDomain.ProcessExit += (_, _) =>
+            // If a connection string is specified (env var, config.json...), always use that.
+            var configured = Config["DefaultConnection"];
+            if (!string.IsNullOrEmpty(configured))
             {
-                try
-                {
-                    container.DisposeAsync().AsTask().GetAwaiter().GetResult();
-                }
-                catch
-                {
-                    // Best-effort cleanup: container may already be stopped or Docker daemon
-                    // may have exited before the process exit handler runs.
-                }
-            };
+                DefaultConnection = configured;
+                _initialized = true;
+                return;
+            }
 
-            var endpoint = new UriBuilder(
-                Uri.UriSchemeHttp,
-                container.Hostname,
-                container.GetMappedPublicPort(CosmosDbBuilder.CosmosDbPort)).ToString();
+            // Try to connect to the default emulator endpoint.
+            if (await TryProbeEmulatorAsync("https://localhost:8081").ConfigureAwait(false))
+            {
+                DefaultConnection = "https://localhost:8081";
+                _initialized = true;
+                return;
+            }
+
+            // Try to start a testcontainer with the Linux emulator.
+            try
+            {
+                _container = new CosmosDbBuilder("mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview")
+                    .Build();
+                await _container.StartAsync().ConfigureAwait(false);
 
-            return (endpoint, container);
+                AppDomain.CurrentDomain.ProcessExit += (_, _) =>
+                {
+                    try
+                    {
+                        _container.DisposeAsync().AsTask().GetAwaiter().GetResult();
+                    }
+                    catch
+                    {
+                        // Best-effort cleanup: container may already be stopped or Docker daemon
+                        // may have exited before the process exit handler runs.
+                    }
+                };
+
+                DefaultConnection = new UriBuilder(
+                    Uri.UriSchemeHttp,
+                    _container.Hostname,
+                    _container.GetMappedPublicPort(CosmosDbBuilder.CosmosDbPort)).ToString();
+                HttpMessageHandler = _container.HttpMessageHandler;
+            }
+            catch when (!SkipConnectionCheck)
+            {
+                // Any failure (Docker not installed, daemon not running, image pull failure, etc.)
+                // falls back to the default endpoint. The connection check in CosmosTestStore will
+                // determine whether the emulator is actually reachable and skip tests if not.
+                DefaultConnection = "https://localhost:8081";
+            }
+
+            _initialized = true;
         }
-        catch
+        finally
         {
-            // Any failure (Docker not installed, daemon not running, image pull failure, etc.)
-            // falls back to the default endpoint. The connection check in CosmosTestStore will
-            // determine whether the emulator is actually reachable and skip tests if not.
-            return ("https://localhost:8081", null);
+            _initSemaphore.Release();
         }
     }
 
-    private static bool TryProbeEmulator(string endpoint)
+    private static async Task<bool> TryProbeEmulatorAsync(string endpoint)
     {
         try
         {
@@ -97,7 +116,7 @@ private static bool TryProbeEmulator(string endpoint)
             };
             using var client = new HttpClient(handler) { Timeout = TimeSpan.FromSeconds(3) };
             // Any successful response (even 401) means the emulator is up and accepting connections.
-            using var response = client.GetAsync(endpoint).GetAwaiter().GetResult();
+            using var response = await client.GetAsync(endpoint).ConfigureAwait(false);
             return true;
         }
         catch
@@ -131,7 +150,7 @@ private static bool TryProbeEmulator(string endpoint)
 
     public static bool SkipConnectionCheck { get; } = string.Equals(Config["SkipConnectionCheck"], "true", StringComparison.OrdinalIgnoreCase);
 
-    public static string EmulatorType => IsTestContainer
+    public static string EmulatorType => _container != null
         ? "linux"
         : Config["EmulatorType"] ?? (!OperatingSystem.IsWindows() ? "linux" : "");
 

Original file line number	Diff line number	Diff line change
`@@ -105,6 +105,8 @@ public override DbContextOptionsBuilder AddProviderOptions(DbContextOptionsBuild`
`105`	`105`
`106`	`106`	`public static async ValueTask<bool> IsConnectionAvailableAsync()`
`107`	`107`	`{`
	`108`	`+ await TestEnvironment.InitializeAsync().ConfigureAwait(false);`
	`109`	`+`
`108`	`110`	`if (TestEnvironment.SkipConnectionCheck)`
`109`	`111`	`{`
`110`	`112`	`return true;`