From e07edaa9d2ca6ba94f788a4a49cdb749b008afdd Mon Sep 17 00:00:00 2001 From: Justin Comins Date: Wed, 26 Jun 2024 14:22:34 -0700 Subject: [PATCH 1/6] nothing to see here --- webui/react/playwright.config.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/webui/react/playwright.config.ts b/webui/react/playwright.config.ts index edaa6104368..165b8daf621 100644 --- a/webui/react/playwright.config.ts +++ b/webui/react/playwright.config.ts @@ -8,10 +8,11 @@ import * as dotenv from 'dotenv'; dotenv.config(); const serverAddess = process.env.PW_SERVER_ADDRESS; -if (serverAddess === undefined) { +const frontEndServerAddess = process.env.PW_FE_SERVER_ADDRESS ?? serverAddess; +if (frontEndServerAddess === undefined) { throw new Error('Expected PW_SERVER_ADDRESS to be set.'); } -const port = Number(new URL(serverAddess).port || 3001); +const port = Number(new URL(frontEndServerAddess).port || 3001); /** * See https://playwright.dev/docs/test-configuration. @@ -81,7 +82,7 @@ export default defineConfig({ /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { actionTimeout: 5_000, - baseURL: `http://localhost:${port}/`, + baseURL: frontEndServerAddess, navigationTimeout: 10_000, trace: 'retain-on-failure', video: 'retain-on-failure', From 2993b13882efcff1224d632c8bb4d327e793e273 Mon Sep 17 00:00:00 2001 From: Justin Comins Date: Thu, 27 Jun 2024 09:19:31 -0700 Subject: [PATCH 2/6] automatic setServerAddress and clean env variables --- webui/react/playwright.config.ts | 13 +++++----- webui/react/src/e2e/fixtures/dev.fixture.ts | 14 ---------- .../react/src/e2e/fixtures/global-fixtures.ts | 26 +++++++++++++++---- webui/react/src/e2e/tests/auth.spec.ts | 3 --- webui/react/src/e2e/utils/detCLI.ts | 3 +-- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/webui/react/playwright.config.ts b/webui/react/playwright.config.ts index 165b8daf621..31758764618 100644 --- a/webui/react/playwright.config.ts +++ b/webui/react/playwright.config.ts @@ -2,17 +2,18 @@ * Read environment variables from file. * https://github.com/motdotla/dotenv */ +import path from 'path'; + import { defineConfig, devices } from '@playwright/test'; import * as dotenv from 'dotenv'; -dotenv.config(); +dotenv.config({ path: path.resolve(__dirname, '.env') }); const serverAddess = process.env.PW_SERVER_ADDRESS; -const frontEndServerAddess = process.env.PW_FE_SERVER_ADDRESS ?? serverAddess; -if (frontEndServerAddess === undefined) { - throw new Error('Expected PW_SERVER_ADDRESS to be set.'); +if (serverAddess === undefined) { + throw new Error(`Expected PW_SERVER_ADDRESS to be set. ${JSON.stringify(process.env)}`); } -const port = Number(new URL(frontEndServerAddess).port || 3001); +const port = Number(new URL(serverAddess).port || 3001); /** * See https://playwright.dev/docs/test-configuration. @@ -82,7 +83,7 @@ export default defineConfig({ /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { actionTimeout: 5_000, - baseURL: frontEndServerAddess, + baseURL: serverAddess, navigationTimeout: 10_000, trace: 'retain-on-failure', video: 'retain-on-failure', diff --git a/webui/react/src/e2e/fixtures/dev.fixture.ts b/webui/react/src/e2e/fixtures/dev.fixture.ts index 816eb6473e5..1f3035cdcc2 100644 --- a/webui/react/src/e2e/fixtures/dev.fixture.ts +++ b/webui/react/src/e2e/fixtures/dev.fixture.ts @@ -1,22 +1,8 @@ -import { Page } from '@playwright/test'; - import { expect } from 'e2e/fixtures/global-fixtures'; import { BaseComponent, CanBeParent } from 'e2e/models/common/base/BaseComponent'; import { BasePage } from 'e2e/models/common/base/BasePage'; export class DevFixture { - readonly #page: Page; - constructor(readonly page: Page) { - this.#page = page; - } - // tells the frontend where to find the backend if built for a different url. Incidentally reloads and logs out of Determined. - async setServerAddress(): Promise { - await this.#page.goto('/'); - await this.#page.evaluate(`dev.setServerAddress("${process.env.PW_SERVER_ADDRESS}")`); - await this.#page.reload(); - await this.#page.waitForLoadState('networkidle'); // dev.setServerAddress fires a logout request in the background, so we will wait until no network traffic is happening. - } - /** * Attempts to locate each element in the locator tree. If there is an error at this step, * the last locator in the error message is the locator that couldn't be found and needs diff --git a/webui/react/src/e2e/fixtures/global-fixtures.ts b/webui/react/src/e2e/fixtures/global-fixtures.ts index d2fa18d8212..69c7c0fa9d7 100644 --- a/webui/react/src/e2e/fixtures/global-fixtures.ts +++ b/webui/react/src/e2e/fixtures/global-fixtures.ts @@ -14,6 +14,7 @@ import { UserFixture } from './user.fixture'; type CustomFixtures = { dev: DevFixture; + devSetup: void; auth: AuthFixture; apiAuth: ApiAuthFixture; user: UserFixture; @@ -30,9 +31,8 @@ type CustomWorkerFixtures = { // https://playwright.dev/docs/test-fixtures export const test = baseTest.extend({ // get the auth but allow yourself to log in through the api manually. - apiAuth: async ({ playwright, browser, dev, baseURL, newAdmin }, use) => { - await dev.setServerAddress(); - const apiAuth = new ApiAuthFixture(playwright.request, browser, baseURL, dev.page); + apiAuth: async ({ playwright, browser, baseURL, newAdmin, page }, use) => { + const apiAuth = new ApiAuthFixture(playwright.request, browser, baseURL, page); await apiAuth.login({ creds: { password: newAdmin.password!, @@ -88,10 +88,26 @@ export const test = baseTest.extend({ }, { scope: 'worker' }, ], - dev: async ({ page }, use) => { - const dev = new DevFixture(page); + // eslint-disable-next-line no-empty-pattern + dev: async ({}, use) => { + const dev = new DevFixture(); await use(dev); }, + devSetup: [ + async ({ page }, use) => { + // Tells the frontend where to find the backend if built for a different url. + // Incidentally reloads and logs out of Determined. + await page.goto('/'); + await page.evaluate( + `dev.setServerAddress("${process.env.DET_WEBPACK_PROXY_URL ?? process.env.PW_SERVER_ADDRESS}")`, + ); + await page.reload(); + // dev.setServerAddress fires a logout request in the background, so we will wait until no network traffic is happening. + await page.waitForLoadState('networkidle'); + await use(); + }, + { auto: true }, + ], /** * Creates an admin and logs in as that admin for the duraction of the test suite */ diff --git a/webui/react/src/e2e/tests/auth.spec.ts b/webui/react/src/e2e/tests/auth.spec.ts index adcbe97459a..60bafd94088 100644 --- a/webui/react/src/e2e/tests/auth.spec.ts +++ b/webui/react/src/e2e/tests/auth.spec.ts @@ -2,9 +2,6 @@ import { expect, test } from 'e2e/fixtures/global-fixtures'; import { SignIn } from 'e2e/models/pages/SignIn'; test.describe('Authentication', () => { - test.beforeEach(async ({ dev }) => { - await dev.setServerAddress(); - }); test.afterEach(async ({ page, auth }) => { const signInPage = new SignIn(page); if ((await page.title()).indexOf(signInPage.title) === -1) { diff --git a/webui/react/src/e2e/utils/detCLI.ts b/webui/react/src/e2e/utils/detCLI.ts index d07a6554b51..052b3317ad9 100644 --- a/webui/react/src/e2e/utils/detCLI.ts +++ b/webui/react/src/e2e/utils/detCLI.ts @@ -10,8 +10,7 @@ export function detExecSync(detCommand: string): string { return execSync(`${process.env.PW_DET_PATH || 'det'} ${detCommand}`, { env: { ...process.env, - DET_MASTER: - process.env.PW_DET_MASTER || process.env.DET_WEBPACK_PROXY_URL || 'http://localhost:8080', + DET_MASTER: process.env.DET_WEBPACK_PROXY_URL || 'http://localhost:8080', DET_PASS: process.env.PW_PASSWORD, DET_USER: process.env.PW_USER_NAME, }, From 43db0a67ade3ce2bf312c648130794b2c8c61010 Mon Sep 17 00:00:00 2001 From: Justin Comins Date: Thu, 27 Jun 2024 13:26:37 -0700 Subject: [PATCH 3/6] placeholder --- .../src/e2e/fixtures/api.auth.fixture.ts | 40 +++++++++++----- webui/react/src/e2e/fixtures/dev.fixture.ts | 13 +++++ .../react/src/e2e/fixtures/global-fixtures.ts | 48 +++++++++---------- .../src/e2e/tests/experimentList.spec.ts | 3 +- webui/react/src/e2e/utils/detCLI.ts | 4 +- webui/react/src/e2e/utils/envVars.ts | 15 ++++++ 6 files changed, 84 insertions(+), 39 deletions(-) create mode 100644 webui/react/src/e2e/utils/envVars.ts diff --git a/webui/react/src/e2e/fixtures/api.auth.fixture.ts b/webui/react/src/e2e/fixtures/api.auth.fixture.ts index 8fe9ccb39df..4ae80999d4e 100644 --- a/webui/react/src/e2e/fixtures/api.auth.fixture.ts +++ b/webui/react/src/e2e/fixtures/api.auth.fixture.ts @@ -1,12 +1,12 @@ import { APIRequest, APIRequestContext, Browser, BrowserContext, Page } from '@playwright/test'; -import { v4 } from 'uuid'; + +import { webServerUrl } from 'e2e/utils/envVars'; export class ApiAuthFixture { apiContext?: APIRequestContext; // we can't get this until login, so may be undefined readonly request: APIRequest; readonly browser: Browser; readonly baseURL: string; - readonly testId = v4(); _page?: Page; get page(): Page { if (this._page === undefined) { @@ -14,11 +14,9 @@ export class ApiAuthFixture { } return this._page; } - readonly #STATE_FILE_SUFFIX = 'state.json'; readonly #USERNAME: string; readonly #PASSWORD: string; - context?: BrowserContext; - readonly #stateFile = `${this.testId}-${this.#STATE_FILE_SUFFIX}`; + browserContext?: BrowserContext; constructor(request: APIRequest, browser: Browser, baseURL?: string, existingPage?: Page) { if (process.env.PW_USER_NAME === undefined) { @@ -56,25 +54,45 @@ export class ApiAuthFixture { * fixture, the bearer token will be attached to that context. If not a new * browser context will be created with the cookie. */ - async login({ + async loginApi({ creds = { password: this.#PASSWORD, username: this.#USERNAME }, } = {}): Promise { - this.apiContext = this.apiContext || (await this.request.newContext()); + this.apiContext = this.apiContext || (await this.request.newContext({ baseURL: this.baseURL })); + const Host = new URL(this.baseURL).hostname + ':' + new URL(this.baseURL).port; const resp = await this.apiContext.post('/api/v1/auth/login', { data: { ...creds, isHashed: false, }, + headers: { + Host, + Origin: webServerUrl(), + Referer: webServerUrl(), + }, }); if (resp.status() !== 200) { + // TODO good breakpoiont for api auth login CORS throw new Error(`Login API request has failed with status code ${resp.status()}`); } + } + + async loginBrowser(): Promise { + if (this.apiContext === undefined) { + throw new Error('Cannot login browser without first logging in API'); + } // Save cookie state into the file. - const state = await this.apiContext.storageState({ path: this.#stateFile }); if (this._page !== undefined) { + const state = await this.apiContext.storageState(); // add cookies to current page's existing context - this.context = this._page.context(); - await this.context.addCookies(state.cookies); + this.browserContext = this._page.context(); + // replace the domain of api base url with browser base url + state.cookies.forEach((cookie) => { + if (cookie.name === 'auth' && cookie.domain === new URL(this.baseURL).hostname) { + cookie.domain = new URL(webServerUrl()).hostname; + } + }); + await this.browserContext.addCookies(state.cookies); + // TODO good breakpoiont for api auth login CORS } } @@ -86,6 +104,6 @@ export class ApiAuthFixture { */ async dispose(): Promise { await this.apiContext?.dispose(); - await this.context?.close(); + await this.browserContext?.close(); } } diff --git a/webui/react/src/e2e/fixtures/dev.fixture.ts b/webui/react/src/e2e/fixtures/dev.fixture.ts index 1f3035cdcc2..af4093e1c3e 100644 --- a/webui/react/src/e2e/fixtures/dev.fixture.ts +++ b/webui/react/src/e2e/fixtures/dev.fixture.ts @@ -1,8 +1,21 @@ +import { Page } from '@playwright/test'; + import { expect } from 'e2e/fixtures/global-fixtures'; import { BaseComponent, CanBeParent } from 'e2e/models/common/base/BaseComponent'; import { BasePage } from 'e2e/models/common/base/BasePage'; +import { apiUrl } from 'e2e/utils/envVars'; export class DevFixture { + setServerAddress = async (page: Page): Promise => { + // Tells the frontend where to find the backend if built for a different url. + // Incidentally reloads and logs out of Determined. + await page.goto('/'); + await page.evaluate(`dev.setServerAddress("${apiUrl()}")`); + await page.reload(); + // dev.setServerAddress fires a logout request in the background, so we will wait until no network traffic is happening. + await page.waitForLoadState('networkidle'); + }; + /** * Attempts to locate each element in the locator tree. If there is an error at this step, * the last locator in the error message is the locator that couldn't be found and needs diff --git a/webui/react/src/e2e/fixtures/global-fixtures.ts b/webui/react/src/e2e/fixtures/global-fixtures.ts index 69c7c0fa9d7..c5015867112 100644 --- a/webui/react/src/e2e/fixtures/global-fixtures.ts +++ b/webui/react/src/e2e/fixtures/global-fixtures.ts @@ -1,10 +1,10 @@ import { expect as baseExpect, test as baseTest, Page } from '@playwright/test'; +import { apiUrl, webServerUrl } from 'e2e/utils/envVars'; import { safeName } from 'e2e/utils/naming'; import { V1PostUserRequest } from 'services/api-ts-sdk/api'; // eslint-disable-next-line no-restricted-imports -import playwrightConfig from '../../../playwright.config'; import { ApiAuthFixture } from './api.auth.fixture'; import { ApiUserFixture } from './api.user.fixture'; @@ -14,7 +14,7 @@ import { UserFixture } from './user.fixture'; type CustomFixtures = { dev: DevFixture; - devSetup: void; + devSetup: Page; auth: AuthFixture; apiAuth: ApiAuthFixture; user: UserFixture; @@ -31,9 +31,9 @@ type CustomWorkerFixtures = { // https://playwright.dev/docs/test-fixtures export const test = baseTest.extend({ // get the auth but allow yourself to log in through the api manually. - apiAuth: async ({ playwright, browser, baseURL, newAdmin, page }, use) => { - const apiAuth = new ApiAuthFixture(playwright.request, browser, baseURL, page); - await apiAuth.login({ + apiAuth: async ({ playwright, browser, newAdmin, devSetup }, use) => { + const apiAuth = new ApiAuthFixture(playwright.request, browser, apiUrl(), devSetup); + await apiAuth.loginApi({ creds: { password: newAdmin.password!, username: newAdmin.user!.username, @@ -53,7 +53,15 @@ export const test = baseTest.extend({ }, // get the existing page but with auth cookie already logged in - authedPage: async ({ apiAuth }, use) => { + authedPage: async ({ apiAuth, auth, newAdmin }, use) => { + if (webServerUrl() === apiUrl()) { + await apiAuth.loginBrowser(); + } else { + await auth.login({ + password: newAdmin.password!, + username: newAdmin.user!.username, + }); + } await use(apiAuth.page); }, @@ -64,12 +72,8 @@ export const test = baseTest.extend({ */ backgroundApiAuth: [ async ({ playwright, browser }, use) => { - const backgroundApiAuth = new ApiAuthFixture( - playwright.request, - browser, - playwrightConfig.use?.baseURL, - ); - await backgroundApiAuth.login(); + const backgroundApiAuth = new ApiAuthFixture(playwright.request, browser, apiUrl()); + await backgroundApiAuth.loginApi(); await use(backgroundApiAuth); await backgroundApiAuth.dispose(); }, @@ -77,7 +81,7 @@ export const test = baseTest.extend({ ], /** * Allows calling the user api without a page so that it can run in beforeAll(). You will need to get a bearer - * token by calling backgroundApiUser.apiAuth.login(). This will also provision a page in the background which + * token by calling backgroundApiUser.apiAuth.loginAPI(). This will also provision a page in the background which * will be disposed of logout(). Before using the page,you need to call dev.setServerAddress() manually and * then login() again, since setServerAddress logs out as a side effect. */ @@ -94,17 +98,9 @@ export const test = baseTest.extend({ await use(dev); }, devSetup: [ - async ({ page }, use) => { - // Tells the frontend where to find the backend if built for a different url. - // Incidentally reloads and logs out of Determined. - await page.goto('/'); - await page.evaluate( - `dev.setServerAddress("${process.env.DET_WEBPACK_PROXY_URL ?? process.env.PW_SERVER_ADDRESS}")`, - ); - await page.reload(); - // dev.setServerAddress fires a logout request in the background, so we will wait until no network traffic is happening. - await page.waitForLoadState('networkidle'); - await use(); + async ({ dev, page }, use) => { + await dev.setServerAddress(page); + await use(page); }, { auto: true }, ], @@ -124,11 +120,11 @@ export const test = baseTest.extend({ }, }), ); - await backgroundApiUser.apiAuth.login({ + await backgroundApiUser.apiAuth.loginApi({ creds: { password: adminUser.password!, username: adminUser.user!.username }, }); await use(adminUser); - await backgroundApiUser.apiAuth.login(); + await backgroundApiUser.apiAuth.loginApi(); await backgroundApiUser.patchUser(adminUser.user!.id!, { active: false }); }, { scope: 'worker' }, diff --git a/webui/react/src/e2e/tests/experimentList.spec.ts b/webui/react/src/e2e/tests/experimentList.spec.ts index 187bc87455d..142eb0caca7 100644 --- a/webui/react/src/e2e/tests/experimentList.spec.ts +++ b/webui/react/src/e2e/tests/experimentList.spec.ts @@ -14,8 +14,9 @@ test.describe('Experiement List', () => { return parseInt(expNum); }; - test.beforeAll(async ({ browser }) => { + test.beforeAll(async ({ browser, dev }) => { const pageSetupTeardown = await browser.newPage(); + await dev.setServerAddress(pageSetupTeardown); const authFixtureSetupTeardown = new AuthFixture(pageSetupTeardown); const projectDetailsPageSetupTeardown = new ProjectDetails(pageSetupTeardown); await authFixtureSetupTeardown.login(); diff --git a/webui/react/src/e2e/utils/detCLI.ts b/webui/react/src/e2e/utils/detCLI.ts index 052b3317ad9..25492e72b18 100644 --- a/webui/react/src/e2e/utils/detCLI.ts +++ b/webui/react/src/e2e/utils/detCLI.ts @@ -1,6 +1,8 @@ import { execSync } from 'child_process'; import path from 'path'; +import { detMasterURL } from './envVars'; + export function fullPath(relativePath: string): string { return path.join(process.cwd(), relativePath); } @@ -10,7 +12,7 @@ export function detExecSync(detCommand: string): string { return execSync(`${process.env.PW_DET_PATH || 'det'} ${detCommand}`, { env: { ...process.env, - DET_MASTER: process.env.DET_WEBPACK_PROXY_URL || 'http://localhost:8080', + DET_MASTER: detMasterURL(), DET_PASS: process.env.PW_PASSWORD, DET_USER: process.env.PW_USER_NAME, }, diff --git a/webui/react/src/e2e/utils/envVars.ts b/webui/react/src/e2e/utils/envVars.ts new file mode 100644 index 00000000000..b57abeb3795 --- /dev/null +++ b/webui/react/src/e2e/utils/envVars.ts @@ -0,0 +1,15 @@ +export function webServerUrl(): string { + const serverAddess = process.env.PW_SERVER_ADDRESS; + if (serverAddess === undefined) { + throw new Error(`Expected PW_SERVER_ADDRESS to be set. ${JSON.stringify(process.env)}`); + } + return serverAddess; +} + +export function apiUrl(): string { + return process.env.PW_BACKEND_SERVER_ADDRESS ?? webServerUrl(); +} + +export function detMasterURL(): string { + return process.env.PW_BACKEND_SERVER_ADDRESS ?? 'localhost:8080'; +} From 532c27b319a8ae8897ac6d1a4a6b7ca079987b62 Mon Sep 17 00:00:00 2001 From: John Kim Date: Thu, 27 Jun 2024 17:08:18 -0400 Subject: [PATCH 4/6] do not require hard-coded row id --- webui/react/src/e2e/tests/experimentList.spec.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/webui/react/src/e2e/tests/experimentList.spec.ts b/webui/react/src/e2e/tests/experimentList.spec.ts index 142eb0caca7..86ae2323f6b 100644 --- a/webui/react/src/e2e/tests/experimentList.spec.ts +++ b/webui/react/src/e2e/tests/experimentList.spec.ts @@ -232,13 +232,13 @@ test.describe('Experiement List', () => { }); test('Datagrid Functionality Validations', async ({ authedPage }) => { - const row = await projectDetailsPage.f_experiemntList.dataGrid.getRowByColumnValue('ID', '1'); + const row = await projectDetailsPage.f_experiemntList.dataGrid.getRowByIndex(0); await test.step('Select Row', async () => { await row.clickColumn('Select'); expect.soft(await row.isSelected()).toBeTruthy(); }); await test.step('Read Cell Value', async () => { - await expect.soft((await row.getCellByColumnName('Checkpoints')).pwLocator).toHaveText('0'); + await expect.soft((await row.getCellByColumnName('ID')).pwLocator).toHaveText(/\d+/); }); await test.step('Select 5', async () => { await ( @@ -247,8 +247,10 @@ test.describe('Experiement List', () => { }); await test.step('Experiement Overview Navigation', async () => { await projectDetailsPage.f_experiemntList.dataGrid.scrollLeft(); + const textContent = await (await row.getCellByColumnName('ID')).pwLocator.textContent(); await row.clickColumn('ID'); - await authedPage.waitForURL(/overview/); + if (textContent === null) throw new Error('Cannot read row id'); + await authedPage.waitForURL(new RegExp(textContent)); }); }); From 4897c5e396fd96e0e47e8f9536fddfe6206baf19 Mon Sep 17 00:00:00 2001 From: Justin Comins Date: Thu, 27 Jun 2024 15:38:24 -0700 Subject: [PATCH 5/6] placeholder --- .../src/e2e/fixtures/api.auth.fixture.ts | 15 +++++--------- .../react/src/e2e/fixtures/global-fixtures.ts | 20 +++++++++---------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/webui/react/src/e2e/fixtures/api.auth.fixture.ts b/webui/react/src/e2e/fixtures/api.auth.fixture.ts index 4ae80999d4e..55275a2a0eb 100644 --- a/webui/react/src/e2e/fixtures/api.auth.fixture.ts +++ b/webui/react/src/e2e/fixtures/api.auth.fixture.ts @@ -36,7 +36,7 @@ export class ApiAuthFixture { this._page = existingPage; } - async getBearerToken(): Promise { + async getBearerToken(noBearer = false): Promise { const cookies = (await this.apiContext?.storageState())?.cookies ?? []; const authToken = cookies.find((cookie) => { return cookie.name === 'auth'; @@ -46,6 +46,7 @@ export class ApiAuthFixture { 'Attempted to retrieve the auth token from the PW apiContext, but it does not exist. Have you called apiAuth.login() yet?', ); } + if (noBearer) return authToken; return `Bearer ${authToken}`; } @@ -58,25 +59,18 @@ export class ApiAuthFixture { creds = { password: this.#PASSWORD, username: this.#USERNAME }, } = {}): Promise { this.apiContext = this.apiContext || (await this.request.newContext({ baseURL: this.baseURL })); - const Host = new URL(this.baseURL).hostname + ':' + new URL(this.baseURL).port; const resp = await this.apiContext.post('/api/v1/auth/login', { data: { ...creds, isHashed: false, }, - headers: { - Host, - Origin: webServerUrl(), - Referer: webServerUrl(), - }, }); if (resp.status() !== 200) { - // TODO good breakpoiont for api auth login CORS throw new Error(`Login API request has failed with status code ${resp.status()}`); } } - async loginBrowser(): Promise { + async loginBrowser(page: Page): Promise { if (this.apiContext === undefined) { throw new Error('Cannot login browser without first logging in API'); } @@ -92,7 +86,8 @@ export class ApiAuthFixture { } }); await this.browserContext.addCookies(state.cookies); - // TODO good breakpoiont for api auth login CORS + const token = JSON.stringify(await this.getBearerToken(true)); + await page.evaluate((token) => localStorage.setItem('global/auth-token', token), token); } } diff --git a/webui/react/src/e2e/fixtures/global-fixtures.ts b/webui/react/src/e2e/fixtures/global-fixtures.ts index c5015867112..10a2a2d914d 100644 --- a/webui/react/src/e2e/fixtures/global-fixtures.ts +++ b/webui/react/src/e2e/fixtures/global-fixtures.ts @@ -1,6 +1,6 @@ import { expect as baseExpect, test as baseTest, Page } from '@playwright/test'; -import { apiUrl, webServerUrl } from 'e2e/utils/envVars'; +import { apiUrl } from 'e2e/utils/envVars'; import { safeName } from 'e2e/utils/naming'; import { V1PostUserRequest } from 'services/api-ts-sdk/api'; @@ -53,15 +53,15 @@ export const test = baseTest.extend({ }, // get the existing page but with auth cookie already logged in - authedPage: async ({ apiAuth, auth, newAdmin }, use) => { - if (webServerUrl() === apiUrl()) { - await apiAuth.loginBrowser(); - } else { - await auth.login({ - password: newAdmin.password!, - username: newAdmin.user!.username, - }); - } + authedPage: async ({ apiAuth }, use) => { + // if (webServerUrl() === apiUrl()) { + await apiAuth.loginBrowser(apiAuth.page); + // } else { + // await auth.login({ + // password: newAdmin.password!, + // username: newAdmin.user!.username, + // }); + // } await use(apiAuth.page); }, From c19df87af674a3df1362a360bf9019ceeadda05e Mon Sep 17 00:00:00 2001 From: Justin Comins Date: Fri, 28 Jun 2024 12:14:19 -0700 Subject: [PATCH 6/6] finalizing env var names --- webui/react/playwright.config.ts | 11 ++++----- webui/react/src/e2e/README.md | 12 ++++++---- .../src/e2e/fixtures/api.auth.fixture.ts | 23 ++++--------------- webui/react/src/e2e/fixtures/auth.fixture.ts | 11 +++------ .../react/src/e2e/fixtures/global-fixtures.ts | 12 ++-------- webui/react/src/e2e/utils/detCLI.ts | 8 +++---- webui/react/src/e2e/utils/envVars.ts | 23 ++++++++++--------- 7 files changed, 37 insertions(+), 63 deletions(-) diff --git a/webui/react/playwright.config.ts b/webui/react/playwright.config.ts index 31758764618..619f6de0911 100644 --- a/webui/react/playwright.config.ts +++ b/webui/react/playwright.config.ts @@ -7,13 +7,12 @@ import path from 'path'; import { defineConfig, devices } from '@playwright/test'; import * as dotenv from 'dotenv'; +import { baseUrl } from 'e2e/utils/envVars'; + dotenv.config({ path: path.resolve(__dirname, '.env') }); -const serverAddess = process.env.PW_SERVER_ADDRESS; -if (serverAddess === undefined) { - throw new Error(`Expected PW_SERVER_ADDRESS to be set. ${JSON.stringify(process.env)}`); -} -const port = Number(new URL(serverAddess).port || 3001); +const baseURL = baseUrl(); +const port = Number(new URL(baseURL).port || 3001); /** * See https://playwright.dev/docs/test-configuration. @@ -83,7 +82,7 @@ export default defineConfig({ /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { actionTimeout: 5_000, - baseURL: serverAddess, + baseURL, navigationTimeout: 10_000, trace: 'retain-on-failure', video: 'retain-on-failure', diff --git a/webui/react/src/e2e/README.md b/webui/react/src/e2e/README.md index bf5803b3750..e9ced4aee37 100644 --- a/webui/react/src/e2e/README.md +++ b/webui/react/src/e2e/README.md @@ -14,11 +14,13 @@ Everything you need before running tests Create `.env` file in `webui/react` like `webui/react/.env` and env variables. (`PW_` prefix stands for Playwright) -- `PW_USER_NAME`: user name for determined account -- `PW_PASSWORD`: password for determined account -- `PW_SERVER_ADDRESS`: API server address -- `PW_DET_PATH`: path to `det` if not already in path -- `PW_DET_MASTER`: typically ", used for CLI commands +- `PW_BASE_URL`: web server tests are pointing to, typically or +- `PW_USERNAME`: admin determined account creds +- `PW_PASSWORD`: admin determined account creds +- `PW_SERVER_ADDRESS`: API server address. defaults to base url +- `PW_DET_PATH`: path to `det`. defaults to `det` +- `PW_DET_MASTER`: used for CLI commands. defaults to +- `PW_EE`: flag for `ee` or `oss`. defaults to unset for oss ### Playwright diff --git a/webui/react/src/e2e/fixtures/api.auth.fixture.ts b/webui/react/src/e2e/fixtures/api.auth.fixture.ts index 55275a2a0eb..45bc144df36 100644 --- a/webui/react/src/e2e/fixtures/api.auth.fixture.ts +++ b/webui/react/src/e2e/fixtures/api.auth.fixture.ts @@ -1,6 +1,6 @@ import { APIRequest, APIRequestContext, Browser, BrowserContext, Page } from '@playwright/test'; -import { webServerUrl } from 'e2e/utils/envVars'; +import { baseUrl, password, username } from 'e2e/utils/envVars'; export class ApiAuthFixture { apiContext?: APIRequestContext; // we can't get this until login, so may be undefined @@ -14,22 +14,9 @@ export class ApiAuthFixture { } return this._page; } - readonly #USERNAME: string; - readonly #PASSWORD: string; browserContext?: BrowserContext; - constructor(request: APIRequest, browser: Browser, baseURL?: string, existingPage?: Page) { - if (process.env.PW_USER_NAME === undefined) { - throw new Error('username must be defined'); - } - if (process.env.PW_PASSWORD === undefined) { - throw new Error('password must be defined'); - } - if (baseURL === undefined) { - throw new Error('baseURL must be defined in playwright config to use API requests.'); - } - this.#USERNAME = process.env.PW_USER_NAME; - this.#PASSWORD = process.env.PW_PASSWORD; + constructor(request: APIRequest, browser: Browser, baseURL: string, existingPage?: Page) { this.request = request; this.browser = browser; this.baseURL = baseURL; @@ -55,9 +42,7 @@ export class ApiAuthFixture { * fixture, the bearer token will be attached to that context. If not a new * browser context will be created with the cookie. */ - async loginApi({ - creds = { password: this.#PASSWORD, username: this.#USERNAME }, - } = {}): Promise { + async loginApi({ creds = { password: password(), username: username() } } = {}): Promise { this.apiContext = this.apiContext || (await this.request.newContext({ baseURL: this.baseURL })); const resp = await this.apiContext.post('/api/v1/auth/login', { data: { @@ -82,7 +67,7 @@ export class ApiAuthFixture { // replace the domain of api base url with browser base url state.cookies.forEach((cookie) => { if (cookie.name === 'auth' && cookie.domain === new URL(this.baseURL).hostname) { - cookie.domain = new URL(webServerUrl()).hostname; + cookie.domain = new URL(baseUrl()).hostname; } }); await this.browserContext.addCookies(state.cookies); diff --git a/webui/react/src/e2e/fixtures/auth.fixture.ts b/webui/react/src/e2e/fixtures/auth.fixture.ts index f7462583985..b85007f3c5c 100644 --- a/webui/react/src/e2e/fixtures/auth.fixture.ts +++ b/webui/react/src/e2e/fixtures/auth.fixture.ts @@ -2,6 +2,7 @@ import { Page } from '@playwright/test'; import { expect } from 'e2e/fixtures/global-fixtures'; import { SignIn } from 'e2e/models/pages/SignIn'; +import { password, username } from 'e2e/utils/envVars'; export class AuthFixture { readonly #page: Page; @@ -10,14 +11,8 @@ export class AuthFixture { readonly signInPage: SignIn; constructor(readonly page: Page) { - if (process.env.PW_USER_NAME === undefined) { - throw new Error('username must be defined'); - } - if (process.env.PW_PASSWORD === undefined) { - throw new Error('password must be defined'); - } - this.#USERNAME = process.env.PW_USER_NAME; - this.#PASSWORD = process.env.PW_PASSWORD; + this.#USERNAME = username(); + this.#PASSWORD = password(); this.#page = page; this.signInPage = new SignIn(page); } diff --git a/webui/react/src/e2e/fixtures/global-fixtures.ts b/webui/react/src/e2e/fixtures/global-fixtures.ts index 10a2a2d914d..61c1739e763 100644 --- a/webui/react/src/e2e/fixtures/global-fixtures.ts +++ b/webui/react/src/e2e/fixtures/global-fixtures.ts @@ -1,6 +1,6 @@ import { expect as baseExpect, test as baseTest, Page } from '@playwright/test'; -import { apiUrl } from 'e2e/utils/envVars'; +import { apiUrl, isEE } from 'e2e/utils/envVars'; import { safeName } from 'e2e/utils/naming'; import { V1PostUserRequest } from 'services/api-ts-sdk/api'; @@ -54,14 +54,7 @@ export const test = baseTest.extend({ // get the existing page but with auth cookie already logged in authedPage: async ({ apiAuth }, use) => { - // if (webServerUrl() === apiUrl()) { await apiAuth.loginBrowser(apiAuth.page); - // } else { - // await auth.login({ - // password: newAdmin.password!, - // username: newAdmin.user!.username, - // }); - // } await use(apiAuth.page); }, @@ -142,8 +135,7 @@ export const expect = baseExpect.extend({ // eslint-disable-next-line @typescript-eslint/no-explicit-any let matcherResult: any; - const isEE = Boolean(JSON.parse(process.env.PW_EE ?? '""')); - const appTitle = isEE ? 'HPE Machine Learning Development Environment' : 'Determined'; + const appTitle = isEE() ? 'HPE Machine Learning Development Environment' : 'Determined'; const getFullTitle = (prefix: string = '') => { if (prefix === '') { diff --git a/webui/react/src/e2e/utils/detCLI.ts b/webui/react/src/e2e/utils/detCLI.ts index 25492e72b18..bf00f1876d3 100644 --- a/webui/react/src/e2e/utils/detCLI.ts +++ b/webui/react/src/e2e/utils/detCLI.ts @@ -1,7 +1,7 @@ import { execSync } from 'child_process'; import path from 'path'; -import { detMasterURL } from './envVars'; +import { detMasterURL, detPath, password, username } from './envVars'; export function fullPath(relativePath: string): string { return path.join(process.cwd(), relativePath); @@ -9,12 +9,12 @@ export function fullPath(relativePath: string): string { export function detExecSync(detCommand: string): string { try { - return execSync(`${process.env.PW_DET_PATH || 'det'} ${detCommand}`, { + return execSync(`${detPath()} ${detCommand}`, { env: { ...process.env, DET_MASTER: detMasterURL(), - DET_PASS: process.env.PW_PASSWORD, - DET_USER: process.env.PW_USER_NAME, + DET_PASS: password(), + DET_USER: username(), }, stdio: 'pipe', timeout: 5_000, diff --git a/webui/react/src/e2e/utils/envVars.ts b/webui/react/src/e2e/utils/envVars.ts index b57abeb3795..eae7640f427 100644 --- a/webui/react/src/e2e/utils/envVars.ts +++ b/webui/react/src/e2e/utils/envVars.ts @@ -1,15 +1,16 @@ -export function webServerUrl(): string { - const serverAddess = process.env.PW_SERVER_ADDRESS; - if (serverAddess === undefined) { - throw new Error(`Expected PW_SERVER_ADDRESS to be set. ${JSON.stringify(process.env)}`); +export function getEnvVar(key: string): string { + const value = process.env[key]; + if (value === undefined) { + throw new Error(`Expected ${key} to be set. ${JSON.stringify(process.env)}`); } - return serverAddess; + return value; } -export function apiUrl(): string { - return process.env.PW_BACKEND_SERVER_ADDRESS ?? webServerUrl(); -} +export const baseUrl = (): string => getEnvVar('PW_BASE_URL'); +export const username = (): string => getEnvVar('PW_USERNAME'); +export const password = (): string => getEnvVar('PW_PASSWORD'); -export function detMasterURL(): string { - return process.env.PW_BACKEND_SERVER_ADDRESS ?? 'localhost:8080'; -} +export const isEE = (): boolean => Boolean(JSON.parse(process.env.PW_EE ?? '""')); +export const apiUrl = (): string => process.env.PW_SERVER_ADDRESS ?? baseUrl(); +export const detMasterURL = (): string => process.env.PW_DET_MASTER ?? 'localhost:8080'; +export const detPath = (): string => process.env.PW_DET_PATH || 'det';