End to End test framework for MSAL 1.0 and 2.0 #1393
Conversation
pkanher617
requested review from
jasonnutter,
DarylThayil,
tnorling,
jmckennon and
sameerag
| "@azure/msal-browser", | ||
| "@azure/msal-angular", | ||
| "vanilla-js-test-app", | ||
| "vanilla-js-test-app-2.0" |
There was a problem hiding this comment.
Can we confirm msal-node works even with dummy tests? Or is that not a concern?
There was a problem hiding this comment.
wait! Is this only browser specific end-to-end?
There was a problem hiding this comment.
I think we can include all end to end tests with this command, I will change the name to reflect this. For msal-browser and msal-core, the end to end tests are all browser based.
There was a problem hiding this comment.
You can add whatever sample runs your node end to end tests when you have it created.
| let username = ""; | ||
| let accountPwd = ""; | ||
|
|
||
| function setupScreenshotDir() { |
There was a problem hiding this comment.
are screenshots checked in?
There was a problem hiding this comment.
no they are ignored
|
|
||
| private async requestLabApi(endpoint: string, accessToken: string): Promise<any> { | ||
| try { | ||
| const response = await axios(`${labApiUri}${endpoint}`, { |
There was a problem hiding this comment.
axios is the client we're also using for node, right? wanted to make sure there was consistency
There was a problem hiding this comment.
yes thats correct, just wanted something simple to use.
| async function enterCredentials(page: puppeteer.Page, testName: string): Promise<void> { | ||
| await page.waitForNavigation({ waitUntil: "networkidle0"}); | ||
| await takeScreenshot(page, testName, `loginPage`); | ||
| await page.type("#i0116", username); | ||
| await page.click("#idSIButton9"); | ||
| await page.waitForNavigation({ waitUntil: "networkidle0"}); | ||
| await takeScreenshot(page, testName, `pwdInputPage`); | ||
| await page.type("#i0118", accountPwd); | ||
| await page.click("#idSIButton9"); | ||
| } |
There was a problem hiding this comment.
since I'm less familiar with puppeteer, my understanding is that this flow is basically:
- navigate to a page
- take a screenshot of the page
- crawl the screenshot to verify data is correct
is this all correct?
There was a problem hiding this comment.
No the screenshot is more of utility thing. The crawling occurs on the headless browser page.
| let browser: puppeteer.Browser; | ||
| before(async () => { | ||
| setupScreenshotDir(); | ||
| browser = await puppeteer.launch(); | ||
| }); | ||
|
|
||
| let context: puppeteer.BrowserContext; | ||
| let page: puppeteer.Page; |
There was a problem hiding this comment.
i'm assuming that the context and page are not declared at the top because their values depend on puppeteer.launch(), right?
There was a problem hiding this comment.
we create a new context and page for each test so we have a fresh environment. browser doesn't need to be reloaded every time.
| let username = ""; | ||
| let accountPwd = ""; | ||
|
|
||
| function setupScreenshotDir() { |
There was a problem hiding this comment.
since this is implemented in most of these test files, can we just implement it in one place and import each time?
There was a problem hiding this comment.
this looks like it could apply to a few functions, like takeScreenshot and potentially setupCredentials
There was a problem hiding this comment.
Yes I am working on creating some test utilities, i will add them as a part of a separate PR.
| await page.waitForNavigation({ waitUntil: "networkidle0"}); | ||
| await takeScreenshot(page, testName, `loginPage`); | ||
| await page.type("#i0116", username); | ||
| await page.click("#idSIButton9"); | ||
| await page.waitForNavigation({ waitUntil: "networkidle0"}); | ||
| await takeScreenshot(page, testName, `pwdInputPage`); | ||
| await page.type("#i0118", accountPwd); | ||
| await page.click("#idSIButton9"); |
There was a problem hiding this comment.
should all of these strings be constants?
There was a problem hiding this comment.
I will enhance some of this in a future PR.
| @@ -19,8 +18,8 @@ const loginRequest = { | |||
|
|
|||
| // Add here the endpoints for MS Graph API services you would like to use. | |||
| const graphConfig = { | |||
| graphMeEndpoint: "Enter_the_Graph_Endpoint_Herev1.0/me", | |||
| graphMailEndpoint: "Enter_the_Graph_Endpoint_Herev1.0/me/messages" | |||
| graphMeEndpoint: "https://graph.microsoft-ppe.com/v1.0/me", | |||
There was a problem hiding this comment.
Should we be using pre-production endpoints?
There was a problem hiding this comment.
We do for 2.0 until we have a test app that works. I am contacting the labs team to set one up for us.
| }); | ||
| return response.data; | ||
| } catch (e) { | ||
| console.error(e); |
There was a problem hiding this comment.
Re-throw instead of returning null?
There was a problem hiding this comment.
I'm going to enhance some of these test utilities in a future PR. for now just getting e2e working.
| import axios from "axios"; | ||
| const labApiUri = "https://msidlab.com/api" | ||
|
|
||
| export class TestCredential { |
There was a problem hiding this comment.
nit: This is more of a LabClient than a TestCredential
There was a problem hiding this comment.
makes sense, I can rename
| const testEnv = envResponse[0]; | ||
| if (testEnv.upn) { | ||
| username = testEnv.upn; | ||
| } |
There was a problem hiding this comment.
If lab response doesn't contain upn, maybe log and throw?
There was a problem hiding this comment.
I'm going to enhance some of these test utilities in a future PR. for now just getting e2e working.
| await page.waitForNavigation({ waitUntil: "networkidle0"}); | ||
| await takeScreenshot(page, testName, `loginPage`); | ||
| await page.type("#i0116", username); | ||
| await page.click("#idSIButton9"); | ||
| await page.waitForNavigation({ waitUntil: "networkidle0"}); | ||
| await takeScreenshot(page, testName, `pwdInputPage`); | ||
| await page.type("#i0118", accountPwd); | ||
| await page.click("#idSIButton9"); |
| let username = ""; | ||
| let accountPwd = ""; | ||
|
|
||
| function setupScreenshotDir() { |
This PR adds an updated end-to-end test framework as well as an updated CLI for samples. It also adds end-to-end tests to the lerna commands as well as to the build pipeline.