Combining or Fixing Test Collections (#3472)

JoshLozensky · web-flow · commit 0c99e5f18165 · 2025-09-09T18:55:20.000-07:00
* use same collection for tests using token acquirer factory

* use same collection for tests using token acquirer factory

* fix UI test collection

* remove file-scoped namespace changes for clarity

* fixing spacing

* formatting changes
diff --git a/tests/E2E Tests/AgentApplications/AgentApplicationsTests.csproj b/tests/E2E Tests/AgentApplications/AgentApplicationsTests.csproj
@@ -16,8 +16,6 @@
   <ItemGroup>
     <PackageReference Include="coverlet.collector" Version="6.0.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
-    <PackageReference Include="xunit" Version="2.5.3" />
-    <PackageReference Include="xunit.runner.visualstudio" Version="2.5.3" />
   </ItemGroup>
 
   <ItemGroup>
@@ -27,6 +25,7 @@
     <ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.GraphServiceClient\Microsoft.Identity.Web.GraphServiceClient.csproj" />
     <ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.OidcFIC\Microsoft.Identity.Web.OidcFIC.csproj" />
     <ProjectReference Include="..\..\..\src\Microsoft.Identity.Web.TokenAcquisition\Microsoft.Identity.Web.TokenAcquisition.csproj" />
+    <ProjectReference Include="..\..\Microsoft.Identity.Web.Test.Common\Microsoft.Identity.Web.Test.Common.csproj" />
   </ItemGroup>
 
   <ItemGroup>
diff --git a/tests/E2E Tests/AgentApplications/GetFicAsyncTests.cs b/tests/E2E Tests/AgentApplications/GetFicAsyncTests.cs
@@ -6,9 +6,11 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Web;
+using Microsoft.Identity.Web.Test.Common;
 
 namespace AgentApplicationsTests
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class GetFicAsyncTests
     {
         [Fact]
diff --git a/tests/E2E Tests/WebAppUiTests/B2CWebAppCallsWebApiLocally.cs b/tests/E2E Tests/WebAppUiTests/B2CWebAppCallsWebApiLocally.cs
@@ -18,7 +18,7 @@ namespace WebAppUiTests
 #if !FROM_GITHUB_ACTION
 {
     // since these tests change environment variables we'd prefer it not run at the same time as other tests
-    [CollectionDefinition(nameof(UiTestNoParallelization), DisableParallelization = true)]
+    [Collection(nameof(UiTestNoParallelization))]
     public class B2CWebAppCallsWebApiLocally : IClassFixture<InstallPlaywrightBrowserFixture>
     {
         private const string KeyvaultEmailName = "IdWeb-B2C-user";
@@ -69,7 +69,7 @@ public async Task Susi_B2C_LocalAccount_TodoAppFunctionsCorrectlyAsync()
             IBrowserContext context = await browser.NewContextAsync(new BrowserNewContextOptions { IgnoreHTTPSErrors = true });
             await context.Tracing.StartAsync(new() { Screenshots = true, Snapshots = true, Sources = true });
 
-            Process? serviceProcess= null;
+            Process? serviceProcess = null;
             Process? clientProcess = null;
 
             try
diff --git a/tests/E2E Tests/WebAppUiTests/TestingWebAppLocally.cs b/tests/E2E Tests/WebAppUiTests/TestingWebAppLocally.cs
@@ -18,7 +18,7 @@ namespace WebAppUiTests;
 #if !FROM_GITHUB_ACTION && !AZURE_DEVOPS_BUILD
 
 // Since this test affects Kestrel environment variables it can cause a race condition when run in parallel with other UI tests.
-[CollectionDefinition(nameof(UiTestNoParallelization), DisableParallelization = true)]
+[Collection(nameof(UiTestNoParallelization))]
 public class TestingWebAppLocally : IClassFixture<InstallPlaywrightBrowserFixture>
 {
     private const string UrlString = "https://localhost:5001/MicrosoftIdentity/Account/signin";
diff --git a/tests/E2E Tests/WebAppUiTests/UiTestHelpers.cs b/tests/E2E Tests/WebAppUiTests/UiTestHelpers.cs
@@ -360,7 +360,7 @@ internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions>
             {
                 if (!UiTestHelpers.ProcessesAreAlive(processes.Values.ToList()))
                 {
-                    RestartProcesses(processes, processDataEntries , output);
+                    RestartProcesses(processes, processDataEntries, output);
                 }
             }
 
@@ -384,7 +384,7 @@ static void RestartProcesses(Dictionary<string, Process> processes, List<Process
                                                     processDataEntry.TestAssemblyLocation,
                                                     processDataEntry.AppLocation,
                                                     processDataEntry.ExecutableName,
-                                                    output, 
+                                                    output,
                                                     processDataEntry.EnvironmentVariables);
                     Thread.Sleep(5000);
 
diff --git a/tests/E2E Tests/WebAppUiTests/WebAppCallsApiCallsGraphLocally.cs b/tests/E2E Tests/WebAppUiTests/WebAppCallsApiCallsGraphLocally.cs
@@ -18,7 +18,7 @@ namespace WebAppUiTests
 #if !FROM_GITHUB_ACTION
 {
     // since these tests change environment variables we'd prefer it not run at the same time as other tests
-    [CollectionDefinition(nameof(UiTestNoParallelization), DisableParallelization = true)]
+    [Collection(nameof(UiTestNoParallelization))]
     public class WebAppCallsApiCallsGraphLocally : IClassFixture<InstallPlaywrightBrowserFixture>
     {
         private const uint GrpcPort = 5001;
@@ -116,7 +116,7 @@ public async Task ChallengeUser_MicrosoftIdFlow_LocalApp_ValidEmailPasswordCreds
                 _output.WriteLine("Starting web app sign-in flow using Todo List button after sign out.");
                 await page.GetByRole(AriaRole.Link, new() { Name = "TodoList" }).ClickAsync();
                 await UiTestHelpers.SuccessiveLogin_MicrosoftIdFlow_ValidEmailPasswordAsync(page, email, labResponse.User.GetOrFetchPassword(), _output);
-                var todoLink =  page.GetByRole(AriaRole.Link, new() { Name = "Create New" });
+                var todoLink = page.GetByRole(AriaRole.Link, new() { Name = "Create New" });
                 await Assertions.Expect(todoLink).ToBeVisibleAsync(_assertVisibleOptions);
                 _output.WriteLine("Web app sign-in flow successful using Todo List button after sign out.");
 
@@ -226,7 +226,7 @@ public async Task ChallengeUser_MicrosoftIdFlow_LocalApp_ValidEmailPasswordCreds
                     foreach (var process in processes)
                     {
 #pragma warning disable CA1305 // Specify IFormatProvider
-                        runningProcesses.AppendLine($"Is {process.Key} running: {UiTestHelpers.ProcessIsAlive(process.Value )}");
+                        runningProcesses.AppendLine($"Is {process.Key} running: {UiTestHelpers.ProcessIsAlive(process.Value)}");
 #pragma warning restore CA1305 // Specify IFormatProvider
                     }
                     Assert.Fail(TC.WebAppCrashedString + " " + runningProcesses.ToString());
diff --git a/tests/E2E Tests/WebAppUiTests/WebAppIntegrationTests.cs b/tests/E2E Tests/WebAppUiTests/WebAppIntegrationTests.cs
@@ -10,9 +10,10 @@
 using Xunit;
 
 namespace WebAppUiTests
-{
 #if !FROM_GITHUB_ACTION
+{
 
+    [Collection(nameof(UiTestNoParallelization))]
     public class WebAppIntegrationTests
     {
         const string UrlString = "https://webapptestmsidweb.azurewebsites.net/MicrosoftIdentity/Account/signin";
@@ -47,6 +48,6 @@ public async Task ChallengeUser_MicrosoftIdentityFlow_RemoteApp_ValidEmailPasswo
             }
 
         }
-#endif //FROM_GITHUB_ACTION
     }
 }
+#endif //FROM_GITHUB_ACTION
diff --git a/tests/Microsoft.Identity.Web.Test/FmiTests.cs b/tests/Microsoft.Identity.Web.Test/FmiTests.cs
@@ -7,13 +7,14 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
+using Microsoft.Identity.Web.Test.Common;
 using Microsoft.Identity.Web.Test.Common.Mocks;
 using Microsoft.Identity.Web.TestOnly;
 using Xunit;
 
 namespace Microsoft.Identity.Web.Test
 {
-    [Collection("Run tests - serial")]
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class FmiTests
     {
         [Fact]
@@ -52,12 +53,12 @@ private TokenAcquirerFactory InitTokenAcquirerFactoryForFmi()
                 options.ClientId = "urn:microsoft:identity:fmi";
                 options.ExtraQueryParameters = new Dictionary<string, string>
                         {
-                                { "dc", "ESTS-PUB-SCUS-LZ1-FD000-TEST1" }
+                            { "dc", "ESTS-PUB-SCUS-LZ1-FD000-TEST1" }
                         };
                 options.ClientCredentials = [ new CredentialDescription() {
                     SourceType = CredentialSource.ClientSecret,
                     ClientSecret = "someSecret"
-                    }];
+                }];
             });
 
             // Add MockedHttpClientFactory
diff --git a/tests/Microsoft.Identity.Web.Test/ManagedIdentityCaeTests.cs b/tests/Microsoft.Identity.Web.Test/ManagedIdentityCaeTests.cs
@@ -9,6 +9,7 @@
 using Microsoft.Identity.Web.TestOnly;
 using Xunit;
 using Microsoft.Identity.Web.Test;
+using Microsoft.Identity.Web.Test.Common;
 using NSubstitute;
 using System.Collections.Generic;
 using System.Net.Http;
@@ -20,7 +21,7 @@
 
 namespace Microsoft.Identity.Web.Tests.Certificateless
 {
-    [Collection("Run tests - serial")]
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class ManagedIdentityTests
     {
         private const string Scope = "https://management.azure.com/.default";
diff --git a/tests/Microsoft.Identity.Web.Test/ServiceCollectionExtensionsTests.cs b/tests/Microsoft.Identity.Web.Test/ServiceCollectionExtensionsTests.cs
@@ -9,10 +9,12 @@
 using Microsoft.Extensions.Options;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
+using Microsoft.Identity.Web.Test.Common;
 using Xunit;
 
 namespace Microsoft.Identity.Web.Test
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class ServiceCollectionExtensionsTests
     {
         [Fact]
diff --git a/tests/Microsoft.Identity.Web.Test/TokenAcquirerCollection.cs b/tests/Microsoft.Identity.Web.Test/TokenAcquirerCollection.cs
diff --git a/tests/Microsoft.Identity.Web.Test/TokenAcquisitionTests.cs b/tests/Microsoft.Identity.Web.Test/TokenAcquisitionTests.cs
@@ -9,13 +9,15 @@
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
+using Microsoft.Identity.Web.Test.Common;
 using Microsoft.Identity.Web.Test.Common.Mocks;
 using Microsoft.Identity.Web.TestOnly;
 using Xunit;
 
 
 namespace Microsoft.Identity.Web.Test
 {
+    [Collection(nameof(TokenAcquirerFactorySingletonProtection))]
     public class TokenAcquisitionTests
     {
         private const string Tenant = "tenant";
@@ -61,10 +63,10 @@ public void TestManagedIdentityWithCommonTenantShouldNotCallResolveTenant()
         {
             // This test verifies that ResolveTenant is not called when using managed identity,
             // which prevents the IDW10405 error when tenant is "common" or "organizations"
-            
+
             // The fix ensures that when ManagedIdentity is specified in tokenAcquisitionOptions,
             // ResolveTenant is skipped entirely, so this scenario should not throw
-            
+
             // Create test options with managed identity
             var tokenOptions = new TokenAcquisitionOptions
             {
@@ -73,15 +75,15 @@ public void TestManagedIdentityWithCommonTenantShouldNotCallResolveTenant()
                     UserAssignedClientId = "test-client-id"
                 }
             };
-            
-            var mergedOptions = new MergedOptions 
-            { 
+
+            var mergedOptions = new MergedOptions
+            {
                 TenantId = Constants.Common  // This would normally cause ResolveTenant to throw
             };
-            
+
             // This should not throw because ResolveTenant should not be called for managed identity scenarios
             // The actual method call would be tested in integration tests, but we can test the logic here
-            
+
             // Verify that ResolveTenant still throws for non-managed identity scenarios
             var exception = Assert.Throws<ArgumentException>(() => TokenAcquisition.ResolveTenant(null, mergedOptions));
             Assert.StartsWith(IDWebErrorMessage.ClientCredentialTenantShouldBeTenanted, exception.Message, StringComparison.Ordinal);
@@ -134,9 +136,9 @@ private TokenAcquirerFactory InitTokenAcquirerFactory()
                 options.TenantId = "f645ad92-e38d-4d1a-b510-d1b09a74a8ca";
                 options.ClientId = "idu773ld-e38d-jud3-45lk-d1b09a74a8ca";
                 options.ExtraQueryParameters = new Dictionary<string, string>
-                        {
-                                { "dc", "ESTS-PUB-SCUS-LZ1-FD000-TEST1" }
-                        };
+                    {
+                        { "dc", "ESTS-PUB-SCUS-LZ1-FD000-TEST1" }
+                    };
                 options.ClientCredentials = [ new CredentialDescription() {
                     SourceType = CredentialSource.ClientSecret,
                     ClientSecret = "someSecret"

Original file line number	Diff line number	Diff line change
`@@ -6,9 +6,11 @@`
`6`	`6`	`using Microsoft.Extensions.DependencyInjection;`
`7`	`7`	`using Microsoft.Identity.Abstractions;`
`8`	`8`	`using Microsoft.Identity.Web;`
	`9`	`+using Microsoft.Identity.Web.Test.Common;`
`9`	`10`
`10`	`11`	`namespace AgentApplicationsTests`
`11`	`12`	`{`
	`13`	`+ [Collection(nameof(TokenAcquirerFactorySingletonProtection))]`
`12`	`14`	`public class GetFicAsyncTests`
`13`	`15`	`{`
`14`	`16`	`[Fact]`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,7 @@ namespace WebAppUiTests;`
`18`	`18`	`#if !FROM_GITHUB_ACTION && !AZURE_DEVOPS_BUILD`
`19`	`19`
`20`	`20`	`// Since this test affects Kestrel environment variables it can cause a race condition when run in parallel with other UI tests.`
`21`		`-[CollectionDefinition(nameof(UiTestNoParallelization), DisableParallelization = true)]`
	`21`	`+[Collection(nameof(UiTestNoParallelization))]`
`22`	`22`	`public class TestingWebAppLocally : IClassFixture<InstallPlaywrightBrowserFixture>`
`23`	`23`	`{`
`24`	`24`	`private const string UrlString = "https://localhost:5001/MicrosoftIdentity/Account/signin";`
Original file line number	Diff line number	Diff line change
`@@ -360,7 +360,7 @@ internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions>`
`360`	`360`	`{`
`361`	`361`	`if (!UiTestHelpers.ProcessesAreAlive(processes.Values.ToList()))`
`362`	`362`	`{`
`363`		`- RestartProcesses(processes, processDataEntries , output);`
	`363`	`+ RestartProcesses(processes, processDataEntries, output);`
`364`	`364`	`}`
`365`	`365`	`}`
`366`	`366`
`@@ -384,7 +384,7 @@ static void RestartProcesses(Dictionary<string, Process> processes, List<Process`
`384`	`384`	`processDataEntry.TestAssemblyLocation,`
`385`	`385`	`processDataEntry.AppLocation,`
`386`	`386`	`processDataEntry.ExecutableName,`
`387`		`- output,`
	`387`	`+ output,`
`388`	`388`	`processDataEntry.EnvironmentVariables);`
`389`	`389`	`Thread.Sleep(5000);`
`390`	`390`
Original file line number	Diff line number	Diff line change
`@@ -10,9 +10,10 @@`
`10`	`10`	`using Xunit;`
`11`	`11`
`12`	`12`	`namespace WebAppUiTests`
`13`		`-{`
`14`	`13`	`#if !FROM_GITHUB_ACTION`
	`14`	`+{`
`15`	`15`
	`16`	`+ [Collection(nameof(UiTestNoParallelization))]`
`16`	`17`	`public class WebAppIntegrationTests`
`17`	`18`	`{`
`18`	`19`	`const string UrlString = "https://webapptestmsidweb.azurewebsites.net/MicrosoftIdentity/Account/signin";`
`@@ -47,6 +48,6 @@ public async Task ChallengeUser_MicrosoftIdentityFlow_RemoteApp_ValidEmailPasswo`
`47`	`48`	`}`
`48`	`49`
`49`	`50`	`}`
`50`		`-#endif //FROM_GITHUB_ACTION`
`51`	`51`	`}`
`52`	`52`	`}`
	`53`	`+#endif //FROM_GITHUB_ACTION`
Original file line number	Diff line number	Diff line change
`@@ -9,10 +9,12 @@`
`9`	`9`	`using Microsoft.Extensions.Options;`
`10`	`10`	`using Microsoft.Identity.Abstractions;`
`11`	`11`	`using Microsoft.Identity.Client;`
	`12`	`+using Microsoft.Identity.Web.Test.Common;`
`12`	`13`	`using Xunit;`
`13`	`14`
`14`	`15`	`namespace Microsoft.Identity.Web.Test`
`15`	`16`	`{`
	`17`	`+ [Collection(nameof(TokenAcquirerFactorySingletonProtection))]`
`16`	`18`	`public class ServiceCollectionExtensionsTests`
`17`	`19`	`{`
`18`	`20`	`[Fact]`