Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Playwright V2 E2E Framework #4706

Merged
merged 12 commits into from
Jul 3, 2024
34 changes: 33 additions & 1 deletion vscode/e2e/utils/vscody/fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,11 @@ const implFixture = _test.extend<TestContext, WorkerContext>({
testInfo,
}
)
const serverExecutableDir = path.join(
executableDir,
path.relative(executableDir, electronExecutable).split(path.sep)[0],
'server'
)

// The location of the executable is platform dependent, try the
// first location that works.
Expand Down Expand Up @@ -437,6 +442,7 @@ const implFixture = _test.extend<TestContext, WorkerContext>({
'serve-web',
'--accept-server-license-terms',
'--port=0',
`--cli-data-dir=${serverExecutableDir.replace(/ /g, '\\ ')}`,
`--server-data-dir=${serverRootDir.replace(/ /g, '\\ ')}`,
`--extensions-dir=${extensionsDir.replace(/ /g, '\\ ')}`, // cli doesn't handle quotes properly so just escape spaces,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be honest I'm surprised you need to escape these strings with that regex given you are using the spawn apis. Are you sure that is needed?

]
Expand Down Expand Up @@ -466,8 +472,34 @@ const implFixture = _test.extend<TestContext, WorkerContext>({
// so we need to do some magic to get the actual port.
// TODO: this might not be very cross-platform
const port = await getPortForPid(codeProcess.pid)

const config = { url: `http://127.0.0.1:${port}/`, token: token }

// we now need to wait for the server to be downloaded
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the server another component that isn't included in the tarball you download? I'm slightly concerned that we are just ignoring what we have put on disk because this cli doesn't see it and it instead is downloading the latest stable version of vscode here.

Is there a global before hook we could maybe be using for all of these? That might be cleaner from a test reporting perspective. But I guess this has some nice properties as well :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing multiple version handling and switching I will get to later. Current setup aligns with how we already electron download latest version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason for not doing global hook is I want to be able to on a test by test basis switch to specific vscode commit if I want to (especially for issue reproductions)

const releaseServerDownloadLock = await waitForLock(serverExecutableDir, {
delay: 1000,
lockfilePath: path.join(serverExecutableDir, '.lock'),
})
try {
stretchTimeout(
async () => {
while (true) {
try {
const res = await fetch(config.url)
if (res.status === 202) {
// we are still downloading here
} else if (res.status === 200 || res.status === 401) {
return
}
} catch {}
await new Promise(resolve => setTimeout(resolve, 1000))
}
},
{ max: 60_000, testInfo }
)
} finally {
releaseServerDownloadLock()
}

await use(config)

// Turn of logging browser logging and navigate away from the UI
Expand Down
Loading