Repository navigation
fix(browser): correctly inherit CLI options - #7858
sheremet-va merged 12 commits into
Conversation
Sorry, something went wrong.
✅ Deploy Preview for vitest-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Sorry, something went wrong.
AriPerkkio
left a comment
There was a problem hiding this comment.
Looks good so far, great work 💯
I wrote couple of manual test cases earlier that fail with these changes:
This config correctly opens Vitest UI when run with vitest --browser.headless=false ✅
import { defineConfig } from "vitest/config";
export default defineConfig({
test: {
browser: {
provider: "playwright",
enabled: true,
headless: true,
instances: [{ browser: "chromium" }],
},
},
});This config opens browser without Vitest UI when run with vitest --browser.headless=false ❌
import { defineConfig } from "vitest/config";
export default defineConfig({
test: {
workspace: [
{
test: {
name: "browser",
browser: {
provider: "playwright",
enabled: true,
headless: true,
instances: [{ browser: "chromium" }],
},
},
},
],
},
});This config does not run test cases in Browser Mode when run with vitest --browser.enabled ❌
import { defineConfig } from "vitest/config";
export default defineConfig({
test: {
workspace: [
{
test: {
name: "browser",
browser: {
provider: "playwright",
enabled: false,
headless: true,
instances: [{ browser: "chromium" }],
},
},
},
],
},
});
Sorry, something went wrong.
| // throw an error to avoid running tests | ||
| throw new Error('stop') |
There was a problem hiding this comment.
This utility looks great - I've thought about building something like this before. Sometimes it's not enough to check resolved options just via resolveConfig as plugins and other parts can change them before tests run. Bailing out like this before pools are running saves time.
Sorry, something went wrong.
I cannot confirm this. All configs you provided open the UI (automatic tests also check that UI is set, tests fail without changes in this PR) |
Sorry, something went wrong.
|
|
||
| if (browser.enabled) { | ||
| // if browser was enabled via CLI and it's configured by the user, then validate the input | ||
| if (browser.enabled && viteConfig.test?.browser) { |
There was a problem hiding this comment.
The main change is that we ignore all browser CLI flags if workspace doesn't have the browser configured
Sorry, something went wrong.
There was a problem hiding this comment.
Note: we throw an error if --browser is provided, but there are no configurations for it
Sorry, something went wrong.
You are right, it seems to work properly now! 💯 |
Sorry, something went wrong.
|
Thanks a million, this will reduce the amount of people acidentally comitting changing headless: true to false because it will be possible to turn it off from the CLI as needed 😌 |
Sorry, something went wrong.
Description
Fixes #7521
Let's complicate config resolution even further!
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
Documentation
Changesets