- Notifications
You must be signed in to change notification settings - Fork925
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 testsI think maybe the PR description is out of date |
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 * as path from "node:path"; | ||
| export default function () { |
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.
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.