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

Remove native auth from MSAL test app #2248

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

Conversation

Yuki-YuXin
Copy link
Contributor

@Yuki-YuXin Yuki-YuXin commented Jan 23, 2025

Introduction

Native auth adopted E2E automation test without any UI. Remove/Revert previous changes to MSAL test app.

Reference

The previous merged related PR, especially the first one.
#2001
#2143
#2118

@Yuki-YuXin Yuki-YuXin requested a review from a team as a code owner January 23, 2025 19:01
Copy link

❌ Work item link check failed. Description does not contain AB#{ID}.

Click here to Learn more.

@Yuki-YuXin Yuki-YuXin added skip AB ID validation No-Changelog This change does not update the changelog. labels Jan 23, 2025
Copy link
Contributor

@shahzaibj shahzaibj left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@nilo-ms nilo-ms left a comment

Choose a reason for hiding this comment

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

This app was added not for E2E tests but to let the native auth team execute (manual) release tests. We didn't automate all manual release tests in E2E, so we still need to use this application. @Yuki-YuXin can you please clarify why we're removing this app?

@Yuki-YuXin
Copy link
Contributor Author

Yuki-YuXin commented Jan 24, 2025

@nilo-ms This part of the code was created before the idea of E2E automation test was born and implemented, during that time all native auth tests relied on manual testing, so we copied a sample app with some changes and put it into the MSAL test app to make it a manual test environment.

For the reason below:

  1. At the moment, E2E automation test covers >80% of our test cases and the remaining <20% can be done entirely using Sample app. Test app are more than 90% replicas of Sample app and the differentiation(attributes) has been covered by automation test.

  2. At that time(2024,Jan), the idea of paving the way for the subsequent automation test existed in the Test app, but after the practice of the E2E automation test, we found that the UI is not necessary for the test of SDK reliability.

  3. No one is maintaining this portion of the Test app code like we did for Sample app (Reference: The last maintenance was in 2024,Jun). Since then we added MFA feature and made MFA private preview release using E2E automation tests without adding or updating Test app for manual test cases. Test app is not compatible/behind with the latest MSAL Native Auth section. In other words, man-made long-term bugs.

  4. In practice, release engineers use Sample app more than Test app because they are separate repository, easy testing by switching branches. However, the Test app is embedded in the MSAL package. Each modification to the Test App is equivalent to a modification to the MSAL package. This partial redundancy is equal to the redundancy of the MSAL package.

If you hold another view on this, I'd be happy to bring it up as a topic in our panel discussion on Thursday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No-Changelog This change does not update the changelog. skip AB ID validation testapps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants