[Issue #11704] set playwright base url and api url explicitly per env - #11800
Conversation
c0e5ead to
2506e6a
Compare
|
|
||
| const apiUrl = API_URLS[targetEnv]; | ||
| const SUPPORTED_ENVS = ["local", "staging"] as const; | ||
| export type SupportedEnvs = (typeof SUPPORTED_ENVS)[number]; |
There was a problem hiding this comment.
I don't think this is necessary for this change, but will be for the next part of the change. I can remove if that's preferred.
| baseUrl: process.env.PLAYWRIGHT_BASE_URL || "http://127.0.0.1:3000", | ||
| apiUrl: process.env.PLAYWRIGHT_API_URL || "http://127.0.0.1:8080", |
There was a problem hiding this comment.
These defaults apply no matter what targetEnv is, right? If someone sets PLAYWRIGHT_TARGET_ENV=staging and doesn't also set PLAYWRIGHT_BASE_URL/PLAYWRIGHT_API_URL, we don't start a web server and we point at localhost while using the staging org label and staging manager key, which could give confusing failures. Would it make sense to have something along these lines?:
const isLocal = targetEnv === "local";
const baseUrl = process.env.PLAYWRIGHT_BASE_URL || (isLocal ? "http://127.0.0.1:3000" : "");
const apiUrl = process.env.PLAYWRIGHT_API_URL || (isLocal ? "http://127.0.0.1:8080" : "");
if (!baseUrl || !apiUrl) {
throw new Error(`PLAYWRIGHT_BASE_URL and PLAYWRIGHT_API_URL must be set when PLAYWRIGHT_TARGET_ENV=${targetEnv}`);
}
There was a problem hiding this comment.
Good point. I was thinking that in local environments we'd always want to use the default, so we'd want to button this up to make sure that all 3 env vars were either set or unset. However, there is a valid use case where you could have targetEnv unset and default to local, but you want to set different port numbers or something, so you'd manually set the url variables. I'm good with this change!
|
|
||
| # Target environment for playwright tests (local or staging) | ||
| PLAYWRIGHT_TARGET_ENV=local | ||
| # Target environment inro for playwright tests |
There was a problem hiding this comment.
| # Target environment inro for playwright tests | |
| # Target environment info for playwright tests |
|
Testing.md could use some updating with these changes as well |
2 participants
Summary
Work for #11704
Changes proposed
Introduces
PLAYWRIGHT_API_URLandPLAYWRIGHT_BASE_URLenvironment variables into the playwright system and CI to allow for directly setting these values without any hard coding. This change is needed to support running tests from the mgmt repo after splitting code.This piece of the work does not address the parts of the ticket oriented towards supporting grantee / grantor test environments. Those pieces are in #11804
Context for reviewers
There should be no change to functionality in this PR.
Validation steps