- Notifications
You must be signed in to change notification settings - Fork1.1k
fix: use pre-built binary instead ofgo run in e2e tests#16236
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
go run in e2e testsspikecurtis commentedJan 23, 2025
I think maybe the PR description is out of date |
buenos-nachos left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Changes look good, but yeah, agree with Spike – definitely need a new description, especially if we're going to be using it for the squashed message
| import{execSync}from"node:child_process"; | ||
| import*aspathfrom"node:path"; | ||
| exportdefaultfunction(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there a reason why this is a default function instead of a named one? Not sure if our setup is looking for default exports specifically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yeah, if you look in playwright.config.ts we specify aglobalSetup option with a path to this file as the value. playwright will then load this file and execute the default export exactly once per test run.
it's not safe to do any of this in the playwright.config.ts file directly because that gets executed once per worker (ie. potentially dozens of times per run)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
aslilac commentedJan 23, 2025
I love that you can see exactly where I realized what the problem was 😂 But yeah, I'll update the description. |
5f4ff58 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Closescoder/internal#204
Closescoder/internal#279
Using
go runinside of a test is fragile, because it means we have to wait forgoto compile the binary while also constrained on resources by the fact that Playwright and coderd are already running. We should instead compile a coder binary for the current platform before the tests and use it directly.