-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix(browser): fix browser testing url for https #4855
Changes from 20 commits
3e54443
4fcabfa
6606019
4068fc2
24b977e
d4b6776
12078db
bcfa016
5df4b5c
c95a42c
bafcd18
4a99736
f5998b7
e49f71c
c16cb13
e6a7259
83f4fcd
4c7caf6
f714731
464801d
029cd3c
8e2348a
b2c79e4
ba594a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -38,7 +38,7 @@ export function createBrowserPool(ctx: Vitest): ProcessPool { | |||||||||||||||||||||||||||||||||||
const provider = project.browserProvider! | ||||||||||||||||||||||||||||||||||||
providers.add(provider) | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const origin = `http://${ctx.config.browser.api?.host || 'localhost'}:${project.browser!.config.server.port}` | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a possibility that this might be null if server is restarted (due to a config change): vitejs/vite#15450 Does it affect us? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the reference. There could be something sketchy scenario. I'll look into it. Also another thing, actually it might not be preferred to use
Let me think about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just remembered this issue about Vite's Essentially Vite does a following when new URL(options.open, resolvedUrls.local[0] ?? resolvedUrls.network[0])
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Regarding Vite server's restart behavior, I don't think that's relevant for Vitest browser mode since browser mode uses a dedicated Vite dev server, which doesn't watch files on its own and is completely re-spawned when Vitest's main server restarts: vitest/packages/vitest/src/integrations/browser/server.ts Lines 21 to 27 in b2c79e4
vitest/packages/vitest/src/node/workspace.ts Lines 271 to 276 in b2c79e4
So, relying on Currently the log looks like this for
But vitest/packages/vitest/src/node/core.ts Line 88 in b2c79e4
vitest/packages/browser/src/node/providers/webdriver.ts Lines 93 to 95 in b2c79e4
|
||||||||||||||||||||||||||||||||||||
const origin = project.browser?.resolvedUrls?.local[0] | ||||||||||||||||||||||||||||||||||||
const paths = files.map(file => relative(project.config.root, file)) | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (project.config.browser.isolate) { | ||||||||||||||||||||||||||||||||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import { expect, test } from "vitest"; | ||
|
||
test("basic", () => { | ||
expect(1).toBe(1); | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
import path from 'node:path' | ||
import { fileURLToPath } from 'node:url' | ||
import { defineConfig } from 'vitest/config' | ||
import basicSsl from '@vitejs/plugin-basic-ssl' | ||
|
||
// test https by | ||
// TEST_HTTPS=1 pnpm test-fixtures --root fixtures/server-url | ||
|
||
const provider = process.env.PROVIDER || 'webdriverio'; | ||
const browser = process.env.BROWSER || (provider === 'playwright' ? 'chromium' : 'chrome'); | ||
|
||
export default defineConfig({ | ||
plugins: [ | ||
!!process.env.TEST_HTTPS && basicSsl(), | ||
], | ||
test: { | ||
browser: { | ||
enabled: true, | ||
provider, | ||
name: browser, | ||
}, | ||
}, | ||
// separate cacheDir from test/browser/vite.config.ts | ||
// to prevent pre-bundling related flakiness on Webkit | ||
cacheDir: path.join(path.dirname(fileURLToPath(import.meta.url)), "node_modules/.vite") | ||
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As commented in #4879 (comment), I found that webkit flakiness is related to Vite pre-bundling somehow. For now, separating vite cacheDir can workaround the issue, so I added this here. To verify this, I repeated I'm still not sure the same issue could be reproduced outside of Vitest since this issue might be related to Vitest being a linked dep for this in-repository tests. |
||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import assert from 'node:assert' | ||
import test from 'node:test' | ||
import { execa } from 'execa' | ||
|
||
test('server-url http', async () => { | ||
const result = await execa('npx', ['vitest', 'run', '--root=./fixtures/server-url', '--browser.headless'], { | ||
env: { | ||
CI: '1', | ||
NO_COLOR: '1', | ||
}, | ||
}) | ||
assert.match(result.stdout, /Browser runner started at http:\/\/localhost:5173\//) | ||
assert.match(result.stdout, /Test Files {2}1 passed/) | ||
}) | ||
|
||
// this test is skipped since browser warns self-signed https and it requires manual interaction. | ||
// you can toggle "skip" to verify it locally. | ||
test('server-url https', { skip: true }, async () => { | ||
const result = await execa('npx', ['vitest', 'run', '--root=./fixtures/server-url'], { | ||
env: { | ||
NO_COLOR: '1', | ||
TEST_HTTPS: '1', | ||
}, | ||
}) | ||
assert.match(result.stdout, /Browser runner started at https:\/\/localhost:5173\//) | ||
assert.match(result.stdout, /Test Files {2}1 passed/) | ||
}) |
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.
Similar to the above
test
job, I thought it's better to run all matrix withoutfail-fast
so that it's easier to identify potential browser-specific quirks.