Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove use of Thread.Sleep #3120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove use of Thread.Sleep #3120

wants to merge 1 commit into from

Conversation

westin-m
Copy link
Contributor

Remove unnecessary usage of Thread.Sleep in the UiTestHelpers class.

@westin-m westin-m requested a review from a team as a code owner October 31, 2024 22:43
@github-actions GitHub Actions
Copy link

Summary

Summary
Generated on: 10/31/2024 - 10:58:37 PM
Coverage date: 10/31/2024 - 10:58:34 PM
Parser: Cobertura
Assemblies: 9
Classes: 158
Files: 166
Line coverage: 51.4% (3177 of 6180)
Covered lines: 3177
Uncovered lines: 3003
Coverable lines: 6180
Total lines: 19826
Branch coverage: 48.4% (1137 of 2348)
Covered branches: 1137
Total branches: 2348
Method coverage: Feature is only available for sponsors

Coverage

Microsoft.Identity.Web - 63.6%
Name Line Branch
Microsoft.Identity.Web 63.6% 56.9%
Microsoft.Identity.Web.AadIssuerValidatorOptions 100%
Microsoft.Identity.Web.AccountExtensions 100% 100%
Microsoft.Identity.Web.AppContextSwitches 100% 100%
Microsoft.Identity.Web.AppServicesAuthenticationBuilderExtensions 100%
Microsoft.Identity.Web.AppServicesAuthenticationHandler 0% 0%
Microsoft.Identity.Web.AppServicesAuthenticationInformation 78.8% 59%
Microsoft.Identity.Web.AppServicesAuthenticationTokenAcquisition 0% 0%
Microsoft.Identity.Web.AuthorityHelpers 100% 92.3%
Microsoft.Identity.Web.AuthorizeForScopesAttribute 8.3% 7.1%
Microsoft.Identity.Web.AzureADB2COpenIDConnectEventHandlers 97.5% 87.5%
Microsoft.Identity.Web.AzureFunctionsAuthenticationHttpContextExtension 0% 0%
Microsoft.Identity.Web.ClaimsPrincipalFactory 100% 100%
Microsoft.Identity.Web.CookiePolicyOptionsExtensions 95.8% 90%
Microsoft.Identity.Web.DefaultMicrosoftIdentityAuthenticationDelegatingHand
lerFactory		 100%
Microsoft.Identity.Web.DownstreamWebApi 10.8% 0%
Microsoft.Identity.Web.DownstreamWebApiExtensions 100%
Microsoft.Identity.Web.DownstreamWebApiGenericExtensions 0% 0%
Microsoft.Identity.Web.DownstreamWebApiOptions 94.7% 50%
Microsoft.Identity.Web.Extensions 100% 100%
Microsoft.Identity.Web.IDownstreamWebApi 0%
Microsoft.Identity.Web.IncrementalConsentAndConditionalAccessHelper 95.8% 87.5%
Microsoft.Identity.Web.MergedOptionsValidation 100% 100%
Microsoft.Identity.Web.MicrosoftIdentityAppAuthenticationMessageHandler 100% 100%
Microsoft.Identity.Web.MicrosoftIdentityAppCallsWebApiAuthenticationBuilder
Extension		 0% 0%
Microsoft.Identity.Web.MicrosoftIdentityAuthenticationBaseMessageHandler 96% 62.5%
Microsoft.Identity.Web.MicrosoftIdentityAuthenticationBaseOptions 100% 50%
Microsoft.Identity.Web.MicrosoftIdentityAuthenticationMessageHandlerHttpCli
entBuilderExtensions		 91.3%
Microsoft.Identity.Web.MicrosoftIdentityAuthenticationMessageHandlerOptions 90%
Microsoft.Identity.Web.MicrosoftIdentityBlazorServiceCollectionExtensions 100%
Microsoft.Identity.Web.MicrosoftIdentityConsentAndConditionalAccessHandler 4.9% 0%
Microsoft.Identity.Web.MicrosoftIdentityServiceHandler 0%
Microsoft.Identity.Web.MicrosoftIdentityUserAuthenticationMessageHandler 100% 66.6%
Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilder 78.9% 0%
Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilderExtensio
ns		 62.2% 27.7%
Microsoft.Identity.Web.MicrosoftIdentityWebApiAuthenticationBuilderWithConf
iguration		 100%
Microsoft.Identity.Web.MicrosoftIdentityWebApiServiceCollectionExtensions 0%
Microsoft.Identity.Web.MicrosoftIdentityWebAppAuthenticationBuilder 94.6% 83.3%
Microsoft.Identity.Web.MicrosoftIdentityWebAppAuthenticationBuilderExtensio
ns		 97% 88.7%
Microsoft.Identity.Web.MicrosoftIdentityWebAppAuthenticationBuilderWithConf
iguration		 81.2%
Microsoft.Identity.Web.MicrosoftIdentityWebAppServiceCollectionExtensions 100%
Microsoft.Identity.Web.PolicyBuilderExtensions 100%
Microsoft.Identity.Web.RequiredScopeExtensions 40%
Microsoft.Identity.Web.RequiredScopeOrAppPermissionExtensions 33.3%
Microsoft.Identity.Web.RequireScopeOptions 100% 50%
Microsoft.Identity.Web.RequireScopeOrAppPermissionOptions 100% 50%
Microsoft.Identity.Web.Resource.JwtBearerMiddlewareDiagnostics 100% 100%
Microsoft.Identity.Web.Resource.MicrosoftIdentityIssuerValidatorFactory 90% 62.5%
Microsoft.Identity.Web.Resource.OpenIdConnectMiddlewareDiagnostics 100% 83.3%
Microsoft.Identity.Web.Resource.RegisterValidAudience 95.6% 90%
Microsoft.Identity.Web.Resource.RequiredScopeAttribute 0%
Microsoft.Identity.Web.Resource.RequiredScopeOrAppPermissionAttribute 0%
Microsoft.Identity.Web.Resource.RolesRequiredHttpContextExtensions 100% 100%
Microsoft.Identity.Web.Resource.ScopesRequiredHttpContextExtensions 100% 100%
Microsoft.Identity.Web.ScopeAuthorizationHandler 90.3% 73%
Microsoft.Identity.Web.ScopeAuthorizationRequirement 100% 50%
Microsoft.Identity.Web.ScopeOrAppPermissionAuthorizationHandler 90.2% 72.9%
Microsoft.Identity.Web.ScopeOrAppPermissionAuthorizationRequirement 78.9% 60%
Microsoft.Identity.Web.TempDataLoginErrorAccessor 15% 25%
Microsoft.Identity.Web.TokenAcquisitionAppTokenCredential 0%
Microsoft.Identity.Web.TokenAcquisitionTokenCredential 0%
Microsoft.Identity.Web.TokenCacheProviders.Session.MsalSessionTokenCachePro
vider		 0% 0%
Microsoft.Identity.Web.TokenCacheProviders.Session.SessionTokenCacheProvide
rExtension		 0% 0%
Microsoft.Identity.Web.Certificate - 41.4%
Name Line Branch
Microsoft.Identity.Web.Certificate 41.4% 26.5%
Microsoft.Identity.Web.Base64EncodedCertificateLoader 90.9% 66.6%
Microsoft.Identity.Web.CertificateDescription 87.9%
Microsoft.Identity.Web.CertificateLoaderHelper 29.1% 20%
Microsoft.Identity.Web.DefaultCertificateLoader 45.6% 31.8%
Microsoft.Identity.Web.DefaultCredentialsLoader 54.7% 50%
Microsoft.Identity.Web.FromPathCertificateLoader 0%
Microsoft.Identity.Web.KeyVaultCertificateLoader 0% 0%
Microsoft.Identity.Web.SignedAssertionFilePathCredentialsLoader 93.7% 100%
Microsoft.Identity.Web.SignedAssertionFromManagedIdentityCredentialLoader 15.7% 0%
Microsoft.Identity.Web.StoreWithDistinguishedNameCertificateLoader 0%
Microsoft.Identity.Web.StoreWithThumbprintCertificateLoader 0%
Microsoft.Identity.Web.Certificateless - 40.1%
Name Line Branch
Microsoft.Identity.Web.Certificateless 40.1% 43.2%
Microsoft.Identity.Web.AzureIdentityForKubernetesClientAssertion 58.1% 63.1%
Microsoft.Identity.Web.CertificatelessOptions 100%
Microsoft.Identity.Web.ClientAssertion 100%
Microsoft.Identity.Web.ClientAssertionProviderBase 100% 80%
Microsoft.Identity.Web.ManagedIdentityClientAssertion 0% 0%
Microsoft.Identity.Web.Diagnostics - 10.2%
Name Line Branch
Microsoft.Identity.Web.Diagnostics 10.2% 7.1%
Microsoft.Identity.Web.Diagnostics.OsHelper 33.3%
Microsoft.Identity.Web.Throws 8.6% 7.1%
Microsoft.Identity.Web.DownstreamApi - 14.5%
Name Line Branch
Microsoft.Identity.Web.DownstreamApi 14.5% 16.1%
Microsoft.Extensions.Configuration.Binder.SourceGeneration 0% 0%
Microsoft.Identity.Web.DownstreamApi 15.4% 21.6%
Microsoft.Identity.Web.DownstreamApiExtensions 51.4% 80%
Microsoft.Identity.Web.DownstreamApiLoggingEventId 100%
System.Runtime.CompilerServices 0%
Microsoft.Identity.Web.MicrosoftGraph - 42%
Name Line Branch
Microsoft.Identity.Web.MicrosoftGraph 42% 4.5%
Microsoft.Identity.Web.BaseRequestExtensions 0% 0%
Microsoft.Identity.Web.GraphServiceCollectionExtensions 82.7% 50%
Microsoft.Identity.Web.MicrosoftGraphExtensions 67.7%
Microsoft.Identity.Web.MicrosoftGraphOptions 100%
Microsoft.Identity.Web.TokenAcquisitionAuthenticationProvider 13.5% 0%
Microsoft.Identity.Web.TokenAcquisitionAuthenticationProviderOption 16.6%
Microsoft.Identity.Web.Test.Common - 69.3%
Name Line Branch
Microsoft.Identity.Web.Test.Common 69.3% 64.5%
Microsoft.Identity.Web.Test.Common.Asserts 100%
Microsoft.Identity.Web.Test.Common.Mocks.LoggerMock`1 50% 100%
Microsoft.Identity.Web.Test.Common.Mocks.MockHttpClientFactory 100% 33.3%
Microsoft.Identity.Web.Test.Common.Mocks.MockHttpContextAccessor 100%
Microsoft.Identity.Web.Test.Common.Mocks.MockHttpCreator 60.8%
Microsoft.Identity.Web.Test.Common.Mocks.MockHttpMessageHandler 76.4% 75%
Microsoft.Identity.Web.Test.Common.Mocks.QueryStringParser 86.8% 85.7%
Microsoft.Identity.Web.Test.Common.Mocks.QueueHttpMessageHandler 57.1% 37.5%
Microsoft.Identity.Web.Test.Common.TestConstants 100%
Microsoft.Identity.Web.Test.Common.TestHelpers.ExternalApp 0% 0%
Microsoft.Identity.Web.Test.Common.TestHelpers.HttpContextUtilities 100%
Microsoft.Identity.Web.Test.Common.TestHelpers.InMemoryTokenCache 0% 0%
Microsoft.Identity.Web.Test.Common.TestHelpers.MsalTestTokenCacheProvider 26.6%
Microsoft.Identity.Web.Test.Common.TestHelpers.TestMsalDistributedTokenCach
eAdapter		 100%
Microsoft.Identity.Web.Test.Common.TestHelpers.TestOptionsMonitor`1 50% 0%
Microsoft.Identity.Web.TokenAcquisition - 53.2%
Name Line Branch
Microsoft.Identity.Web.TokenAcquisition 53.2% 55.1%
Microsoft.Identity.Web.ApplicationBuilderExtensions 0%
Microsoft.Identity.Web.AuthCodeRedemptionParameters 100%
Microsoft.Identity.Web.CiamAuthorityHelper 100% 92.8%
Microsoft.Identity.Web.ClientInfo 100% 87.5%
Microsoft.Identity.Web.ConfidentialClientApplicationBuilderExtension 79% 60.5%
Microsoft.Identity.Web.ConfidentialClientApplicationOptionsMerger 100% 50%
Microsoft.Identity.Web.DefaultAuthorizationHeaderProvider 54.9% 26.6%
Microsoft.Identity.Web.DefaultTokenAcquirerFactoryImplementation 1.6% 0%
Microsoft.Identity.Web.Experimental.CertificateChangeEventArg 0%
Microsoft.Identity.Web.Extensibility.BaseAuthorizationHeaderProvider 45.4%
Microsoft.Identity.Web.Hosts.DefaultTokenAcquisitionHost 38.4% 0%
Microsoft.Identity.Web.HttpContextExtensions 100%
Microsoft.Identity.Web.IdHelper 95.8% 50%
Microsoft.Identity.Web.Internal.WebApiBuilders 100% 100%
Microsoft.Identity.Web.ITokenAcquisition 0%
Microsoft.Identity.Web.JwtBearerOptionsMerger 0% 0%
Microsoft.Identity.Web.LoggingEventId 100%
Microsoft.Identity.Web.LoggingOptions 100%
Microsoft.Identity.Web.MergedOptions 93.3% 92%
Microsoft.Identity.Web.MergedOptionsStore 100%
Microsoft.Identity.Web.MicrosoftIdentityAppCallsWebApiAuthenticationBuilder 83.3% 100%
Microsoft.Identity.Web.MicrosoftIdentityApplicationOptionsMerger 60% 0%
Microsoft.Identity.Web.MicrosoftIdentityBaseAuthenticationBuilder 85.1% 78.5%
Microsoft.Identity.Web.MicrosoftIdentityOptions 100% 75%
Microsoft.Identity.Web.MicrosoftIdentityOptionsMerger 100% 50%
Microsoft.Identity.Web.MicrosoftIdentityWebChallengeUserException 0%
Microsoft.Identity.Web.MsalAspNetCoreHttpClientFactory 50%
Microsoft.Identity.Web.MsAuth10AtPop 100% 100%
Microsoft.Identity.Web.PrincipalExtensionsForSecurityTokens 100% 70%
Microsoft.Identity.Web.ServiceCollectionExtensions 66.6% 64.2%
Microsoft.Identity.Web.TokenAcquirer 0% 0%
Microsoft.Identity.Web.TokenAcquirerFactory 55.4% 50%
Microsoft.Identity.Web.TokenAcquisition 22.9% 17.2%
Microsoft.Identity.Web.TokenAcquisitionAspNetCore 4.6% 0%
Microsoft.Identity.Web.TokenAcquisitionAspnetCoreHost 40.4% 23.6%
Microsoft.Identity.Web.TokenAcquisitionExtensionOptions 100% 100%
Microsoft.Identity.Web.TokenAcquisitionOptions 100%
Microsoft.Identity.Web.Util.Base64UrlHelpers 93.4% 81.4%
Microsoft.Identity.Web.TokenCache - 80.8%
Name Line Branch
Microsoft.Identity.Web.TokenCache 80.8% 82.6%
Microsoft.Identity.Web.ClaimsPrincipalExtensions 96.7% 100%
Microsoft.Identity.Web.LoggingEventId 100%
Microsoft.Identity.Web.TokenCacheExtensions 100%
Microsoft.Identity.Web.TokenCacheProviders.CacheSerializerHints 100%
Microsoft.Identity.Web.TokenCacheProviders.Distributed.DistributedTokenCach
eAdapterExtension		 100%
Microsoft.Identity.Web.TokenCacheProviders.Distributed.MsalDistributedToken
CacheAdapter		 83% 83.3%
Microsoft.Identity.Web.TokenCacheProviders.Distributed.MsalDistributedToken
CacheAdapterOptions		 88.8%
Microsoft.Identity.Web.TokenCacheProviders.InMemory.InMemoryTokenCacheProvi
derExtension		 100%
Microsoft.Identity.Web.TokenCacheProviders.InMemory.MsalMemoryTokenCacheOpt
ions		 100%
Microsoft.Identity.Web.TokenCacheProviders.InMemory.MsalMemoryTokenCachePro
vider		 83.8% 81.8%
Microsoft.Identity.Web.TokenCacheProviders.MeasureDurationResult 100%
Microsoft.Identity.Web.TokenCacheProviders.MeasureDurationResult`1 0%
Microsoft.Identity.Web.TokenCacheProviders.MsalAbstractTokenCacheProvider 50.7% 66.6%
Microsoft.Identity.Web.TokenCacheProviders.Utility 54.5%

msbw2
msbw2 reviewed Oct 31, 2024
View reviewed changes
tests/E2E Tests/WebAppUiTests/UiTestHelpers.cs
@@ -324,7 +324,6 @@ internal static bool StartAndVerifyProcessesAreRunning(List<ProcessStartOptions>
processDataEntry.EnvironmentVariables);

processes.Add(processDataEntry.ExecutableName, process);
Thread.Sleep(5000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to block the thread on waiting for all processes to start or error? I assume that's the intent behind this sleep.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can check that all processes have started, you can poll on that condition. Like here https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/main/tests/Microsoft.Identity.Test.Common/TestCommon.cs#L258

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, it looks like something similar is being done with these tests, after the processes are checked to be running

microsoft-identity-web/tests/E2E Tests/WebAppUiTests/WebAppCallsApiCallsGraphLocally.cs

Line 335 in 011bd15

private async Task<IPage> NavigateToWebAppAsync(IBrowserContext context, uint port)

msbw2
msbw2 reviewed Oct 31, 2024
View reviewed changes
tests/E2E Tests/WebAppUiTests/UiTestHelpers.cs
@@ -357,7 +356,6 @@ static void RestartProcesses(Dictionary<string, Process> processes, List<Process
processDataEntry.AppLocation,
processDataEntry.ExecutableName,
processDataEntry.EnvironmentVariables);
Thread.Sleep(5000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

jmprieur
jmprieur reviewed Nov 1, 2024
View reviewed changes
Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @westin-m
Did you try this? I think we need to let some time for the web APIs to start?

@westin-m westin-m linked an issue Nov 4, 2024 that may be closed by this pull request
[IdWeb] Remove uses of Thread.Sleep in tests #3126
Open
@westin-m
Copy link
Contributor Author

westin-m commented Nov 4, 2024

Did you try this? I think we need to let some time for the web APIs to start?

I did try it and it worked when ran until failure. Nevertheless, I'll find a way to wait for all processes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msbw2 msbw2 msbw2 left review comments

@jmprieur jmprieur jmprieur left review comments

@bgavrilMS bgavrilMS bgavrilMS left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[IdWeb] Remove uses of Thread.Sleep in tests
4 participants
@westin-m @bgavrilMS @jmprieur @msbw2