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.

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

Refactored UI Tests #3039

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

Refactored UI Tests #3039

wants to merge 1 commit into from

Conversation

JoshLozensky
Copy link
Contributor

Description

Refactored UI tests to be more uniform in implementation so it is easier to maintain and understand.

Additionally, removed ConfigureAwait(false) from async test calls as XUnit guidance suggests this can negatively impact concurrence as described here. (I'll issue a separate PR suggesting removal from all Id Web tests).

@JoshLozensky JoshLozensky requested a review from a team as a code owner September 26, 2024 00:31
@@ -46,7 +49,7 @@ public async Task Susi_B2C_LocalAccount_TodoAppFucntionsCorrectly()
{
// Web app and api environmental variable setup.
DefaultAzureCredential azureCred = new();
string clientSecret = await UiTestHelpers.GetValueFromKeyvaultWitDefaultCreds(_keyvaultUri, KeyvaultClientSecretName, azureCred);
string clientSecret = await UITH.GetValueFromKeyvaultWitDefaultCreds(_keyvaultUri, KeyvaultClientSecretName, azureCred);
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest keeping UiTestHelpers naming. UITH is not a common acronym, so it's not immediately obvious what it represents - makes code less readable. Plus with IntelliSense auto-complete, the name length is not a huge issue.

@JoshLozensky JoshLozensky marked this pull request as draft September 27, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants