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

feat(debug): support debugging with headed browser #603

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

nCastle1
Copy link

@nCastle1 nCastle1 commented Mar 21, 2025

Fixes #474

This PR improves support for debugging in headless browser scenarios by automatically setting up an additional launch configuration to run after starting debugging. This enables connection to the browser running the tests, which enables hitting breakpoints. This is done only when browser mode is enabled.

Currently debugging support is limited to chrome via Playwright and webdriverio; additional setup needed for webdriverio.

Implementation details: after the debug session starts, if vitest's resolved config includes a browser entry, a second debug session is launched as a child of the first. Stopping the parent debug session or the child session will stop the other. The CLI arg to start the vitest debug run are manipulated as needed to make debugging work, and this is different for playwright and webdriverio.

Vitest fails if called with --inspect-brk when using webdriverio. A future area of inquiry might involve updating the inspect-brk implementation to work seamlessly with webdriverio.

I have included sample projects for a few scenarios impacted by the changes in this PR.

This is my first time contributing to this repo. I checked for contribution guidelines but didn't find anything beyond the code of conduct; please let me know if you have any other guidance.

This commit improves support for debugging in non-headless browser scenarios by adding the option to define an additional launch configuration to run after starting debugging. This enables connection to the browser running the tests, which enables setting breakpoints. This addresses vitest-dev#474.

Implementation details: after the debug session starts, if configured, a second debug session is launched as a child of the first. I confirmed by inspection of VS Code source that child debug sessions are stopped when the parent is stopped, so I don't think additional clean up is needed. ([Evidence](https://github.com/microsoft/vscode/blob/3286791a7e920c9b61b84fc2dbba136da912bb64/src/vs/workbench/contrib/debug/browser/debugSession.ts#L426))

I have included a sample project that demonstrates how to configure headful browser tests; it is copy-pasted version of the browser sample, with some config changes. I have also updated the readme to document this feature.
@nCastle1 nCastle1 marked this pull request as draft March 24, 2025 17:20
This is useful because sometimes you want to use different arguments
to vitest when debugging. In particular, you don't want to pass
inspect-brk when not debugging, but you need to pass that for
browser-based debugging.
Automatically stop the debugging session when the secondary session is
stopped; otherwise user needs to press stop button twice.
@sheremet-va
Copy link
Member

sheremet-va commented Mar 25, 2025

Thank you for PR! Looks very good, but I don't think we should require any new additional config options. We can infer them from the config ourselves.

@nCastle1
Copy link
Author

Thank you for that feedback, I will re-work the PR to infer as much as possible from the existing config.

- Removes existing implementation using explicit config options
- Updates the rpc/worker to support reading vite's resolved browser
configuration. This information is used to determine when the
secondary attach config should be started. This information is also
used to set the inspect-brk and browser flags on the vitest CLI when
starting the debug session. This enables zero-config debugging when
the user has started a test debug without interfering with running
without debug. inspect-brk is used to guarantee that breakpoints
early in the test run are hit (otherwise it is possible tests run
before the debugger is attached).
- Adds samples which verify that
this feature works correctly when there are multiple instances
configured and when webdriverio is being used (note: debugging is not
supported for webdriverio, this just makes sure the feature doesn't
break test running).

Only chromium with playwright is supported by the current
implementation.
Not a great solution because it relies on additional configuration
being done at the project level, including setting up custom command
to communicate with webdriverio (this is that project's promoted
pattern).
@@ -183,6 +217,24 @@ async function getRuntimeOptions(pkg: VitestPackage) {
}
}

function getAttachConfigForBrowser(config: ReturnType<typeof getConfig>, pkg: VitestPackage): vscode.DebugConfiguration {
Copy link
Author

Choose a reason for hiding this comment

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

I split this out originally with intent to support multiple browsers, but I'm not sure how feasible that is. Vitest only docs playwright with chromium for the vs code/IDE-integrated debugging. Firefox doesn't support inspect-brk flag, and the VS Code integration has some limitations that will make integration challenging - if the browser instance isn't running at attach time, the attach config fails immediately instead of waiting like the built-in chrome attach implementation.

Copy link
Member

Choose a reason for hiding this comment

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

I'd say if it doesn't work in Vitest CLI, the extension cannot add support for it, and waiting for it is a bit useless

It's better to throw an error, maybe? Or a warning?

@@ -146,6 +146,10 @@ export class ExtensionWorker implements ExtensionWorkerTransport {
return files.map(([project, spec]) => [project.config.name || '', spec])
}

public getResolvedBrowserOptions(): ResolvedBrowserOptions | undefined {
return { ...this.ctx.config?.browser, commands: undefined }
Copy link
Author

Choose a reason for hiding this comment

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

This needs to be reworked, complex configuration (e.g. custom commands) will break the extension currently.

Copy link
Member

Choose a reason for hiding this comment

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

I think you can just rename this to getBrowserDebugOptions and return only needed fields

@nCastle1
Copy link
Author

I've updated the PR to take a zero configuration approach, reading the browser config as resolved by vitest. I have it working with playwright and chromium; this seems pretty solid. I'm working towards a solution for webdriverio, but I have zero experience with that technology so it is slow going.

I've noted a few key missing pieces, and I don't think this is in a mergable state, so I will keep as a draft. I will plan to revisit and add tests soon (pending some competing deadlines). Any feedback is very welcome in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants