test: env variable config and automatic setServerAddress [TESTENG-49] #9582

JComins000 · 2024-06-27T16:20:58Z

Ticket

TESTENG-49

Description

autouse dev setserveradress
move env var config to a file
rename env vars, check the readme

Test Plan

Checklist

Changes have been manually QA'd
New features have been approved by the corresponding PM
User-facing API changes have the "User-facing API Change" label
Release notes have been added as a separate file under docs/release-notes/
See Release Note for details.
Licenses have been included for new code which was copied and/or modified from any external code

netlify · 2024-06-27T16:21:14Z

✅ Deploy Preview for determined-ui ready!

Name	Link
🔨 Latest commit	`c19df87`
🔍 Latest deploy log	https://app.netlify.com/sites/determined-ui/deploys/667f2468592a5b0008ff45d5
😎 Deploy Preview	https://deploy-preview-9582--determined-ui.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

JComins000 · 2024-06-27T16:21:20Z

webui/react/playwright.config.ts


 const serverAddess = process.env.PW_SERVER_ADDRESS;
 if (serverAddess === undefined) {
-  throw new Error('Expected PW_SERVER_ADDRESS to be set.');
+  throw new Error(`Expected PW_SERVER_ADDRESS to be set. ${JSON.stringify(process.env)}`);


circle automatically hides sensitive things

codecov · 2024-06-27T16:23:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.27%. Comparing base (b73fbbc) to head (c19df87).
Report is 17 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9582      +/-   ##
==========================================
- Coverage   49.86%   45.27%   -4.60%     
==========================================
  Files        1247      923     -324     
  Lines      162287   121954   -40333     
  Branches     2888     2887       -1     
==========================================
- Hits        80931    55216   -25715     
+ Misses      81185    66567   -14618     
  Partials      171      171

Flag	Coverage Δ
harness	`?`
web	`46.22% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

see 324 files with indirect coverage changes

JComins000 · 2024-06-27T16:23:49Z

webui/react/src/e2e/fixtures/global-fixtures.ts

    await use(dev);
  },
+  devSetup: [


now we automatically use a dev command on the UI for each page we get (every test). the dev command is set to local storage

JComins000 · 2024-06-27T22:39:31Z

webui/react/src/e2e/fixtures/api.auth.fixture.ts

+      });
+      await this.browserContext.addCookies(state.cookies);
+      const token = JSON.stringify(await this.getBearerToken(true));
+      await page.evaluate((token) => localStorage.setItem('global/auth-token', token), token);


this was the magic i needed

JComins000 · 2024-06-27T22:43:12Z

webui/react/src/e2e/utils/envVars.ts

+}
+
+export function apiUrl(): string {
+  return process.env.PW_BACKEND_SERVER_ADDRESS ?? webServerUrl();


@djanicekpach @johnkim-det can you help me reconsider this environment variable name? maybe like this?

PW_BASE_URL="http://localhost:3000/" PW_SERVER_ADDRESS="http://latest-main.determined.ai:8080/" PW_USER_NAME="..." PW_PASSWORD="..." PW_DET_PATH="/Users/jcomins/miniconda3/envs/det/bin/det"

where PW_SERVER_ADDRESS would be optional, so this would also work

PW_BASE_URL="http://localhost:3000" PW_USER_NAME="admin" PW_PASSWORD="" PW_DET_PATH="/Users/jcomins/miniconda3/envs/det/bin/det"

do these names look good to you?

I saw PW_DET_MASTER in code and I thought that name was good. Is that included?
I don't love PW_BASE_URL, but it matches playwright's config so I'm ok with it.

JComins000 · 2024-06-27T22:43:31Z

webui/react/playwright.config.ts

 import { defineConfig, devices } from '@playwright/test';
 import * as dotenv from 'dotenv';

-dotenv.config();
+dotenv.config({ path: path.resolve(__dirname, '.env') });


@johnkim-det can you try .env with this change?

webui/react/src/e2e/fixtures/global-fixtures.ts

djanicekpach · 2024-06-28T19:55:31Z

webui/react/src/e2e/utils/envVars.ts

+}
+
+export function apiUrl(): string {
+  return process.env.PW_BACKEND_SERVER_ADDRESS ?? webServerUrl();


I saw PW_DET_MASTER in code and I thought that name was good. Is that included?
I don't love PW_BASE_URL, but it matches playwright's config so I'm ok with it.

JComins000 added 2 commits June 26, 2024 14:22

nothing to see here

e07edaa

automatic setServerAddress and clean env variables

2993b13

JComins000 requested a review from a team as a code owner June 27, 2024 16:20

cla-bot bot added the cla-signed label Jun 27, 2024

JComins000 commented Jun 27, 2024

View reviewed changes

placeholder

43db0a6

JComins000 force-pushed the jcom/temp/testing-pw-config-stuff branch from 986cc2e to 43db0a6 Compare June 27, 2024 20:27

johnkim-det and others added 2 commits June 27, 2024 17:08

do not require hard-coded row id

532c27b

placeholder

4897c5e

JComins000 commented Jun 27, 2024

View reviewed changes

johnkim-det self-requested a review June 28, 2024 17:14

johnkim-det self-assigned this Jun 28, 2024

JComins000 force-pushed the jcom/temp/testing-pw-config-stuff branch 5 times, most recently from a08cd71 to cf9f024 Compare June 28, 2024 19:54

djanicekpach approved these changes Jun 28, 2024

View reviewed changes

JComins000 force-pushed the jcom/temp/testing-pw-config-stuff branch from cf9f024 to 78b3c2a Compare June 28, 2024 20:02

finalizing env var names

c19df87

JComins000 force-pushed the jcom/temp/testing-pw-config-stuff branch from 78b3c2a to c19df87 Compare June 28, 2024 21:00

JComins000 merged commit 98d574e into main Jul 1, 2024
85 of 98 checks passed

JComins000 deleted the jcom/temp/testing-pw-config-stuff branch July 1, 2024 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: env variable config and automatic setServerAddress [TESTENG-49] #9582

test: env variable config and automatic setServerAddress [TESTENG-49] #9582

JComins000 commented Jun 27, 2024 •

edited

Loading

netlify bot commented Jun 27, 2024 •

edited

Loading

JComins000 Jun 27, 2024

codecov bot commented Jun 27, 2024 •

edited

Loading

JComins000 Jun 27, 2024 •

edited

Loading

JComins000 Jun 27, 2024

JComins000 Jun 27, 2024

djanicekpach Jun 28, 2024

JComins000 Jun 27, 2024

djanicekpach Jun 28, 2024

test: env variable config and automatic setServerAddress [TESTENG-49] #9582

test: env variable config and automatic setServerAddress [TESTENG-49] #9582

Conversation

JComins000 commented Jun 27, 2024 • edited Loading

Ticket

Description

Test Plan

Checklist

netlify bot commented Jun 27, 2024 • edited Loading

✅ Deploy Preview for determined-ui ready!

JComins000 Jun 27, 2024

Choose a reason for hiding this comment

codecov bot commented Jun 27, 2024 • edited Loading

Codecov Report

JComins000 Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

JComins000 Jun 27, 2024

Choose a reason for hiding this comment

JComins000 Jun 27, 2024

Choose a reason for hiding this comment

djanicekpach Jun 28, 2024

Choose a reason for hiding this comment

JComins000 Jun 27, 2024

Choose a reason for hiding this comment

djanicekpach Jun 28, 2024

Choose a reason for hiding this comment

JComins000 commented Jun 27, 2024 •

edited

Loading

netlify bot commented Jun 27, 2024 •

edited

Loading

codecov bot commented Jun 27, 2024 •

edited

Loading

JComins000 Jun 27, 2024 •

edited

Loading