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

fix: server.address before listen, chdir in test, basic cli test #5059

Merged
merged 17 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
72c18a0
test(cli): add playground with tests for cli
dominikg Sep 23, 2021
5cf68de
fix: improve check condition if cli process kill was successful
dominikg Sep 25, 2021
d59d700
fix: use detached process for cli server and kill it with native comm…
dominikg Sep 25, 2021
9981286
fix: remove unused timeout param and increase start timeout to 5s
dominikg Sep 25, 2021
4b43e29
wip: see if ipc works
dominikg Sep 25, 2021
fc29624
wip: remove detached: true. only send additional taskkill command on …
dominikg Sep 25, 2021
9f2f178
wip: add --strict-port
dominikg Sep 26, 2021
11c2183
refactor: remove nested finally and add back error log for failed ser…
dominikg Sep 26, 2021
e58be2a
wip: log commands used on error
dominikg Sep 26, 2021
fecf5da
wip: revert to state that ran successfully on linux/mac, add extra ta…
dominikg Sep 26, 2021
0a3c3a3
wip: don't rely on execa preferLocal, call vite bin with relative path
dominikg Sep 26, 2021
0c5218d
fix: use viteBin for server process too
dominikg Sep 26, 2021
9a02e48
fix: refactor postcss-caching test to not call process.chdir and sile…
dominikg Sep 26, 2021
6b54ce7
fix: enforce localhost for cli test and avoid logger error when tryin…
dominikg Sep 26, 2021
29dd8d1
fix: call server.listen() before accessing server.address() to get th…
dominikg Sep 26, 2021
bdd36bd
wip: and another round of windows improvement
dominikg Sep 26, 2021
76b59b9
refactor: cleanup serve implementation in cli playground, improve err…
dominikg Sep 26, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/playground/cli/__tests__/cli.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { port } from './serve'

test('cli should work', async () => {
// this test uses a custom serve implementation, so regular helpers for browserLogs and goto don't work
// do the same thing manually
const logs = []
const onConsole = (msg) => {
logs.push(msg.text())
}
try {
page.on('console', onConsole)
await page.goto(`http://localhost:${port}/`)
expect(await page.textContent('.app')).toBe('vite cli works!')
expect(logs.some((msg) => msg.match('vite cli works!'))).toBe(true)
} finally {
page.off('console', onConsole)
}
})
166 changes: 166 additions & 0 deletions packages/playground/cli/__tests__/serve.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
// @ts-check
// this is automtically detected by scripts/jestPerTestSetup.ts and will replace
// the default e2e test serve behavior

const path = require('path')
// eslint-disable-next-line node/no-restricted-require
const execa = require('execa')
const { workspaceRoot } = require('../../testUtils')

const isWindows = process.platform === 'win32'
const port = (exports.port = 9510) // make sure this port is unique across tests with custom servers
const viteBin = path.join(workspaceRoot, 'packages', 'vite', 'bin', 'vite.js')

/**
* @param {string} root
* @param {boolean} isProd
*/
exports.serve = async function serve(root, isProd) {
// collect stdout and stderr streams from child processes here to avoid interfering with regular jest output
const streams = {
build: { out: [], err: [] },
server: { out: [], err: [] }
}
// helpers to collect streams
const collectStreams = (name, process) => {
process.stdout.on('data', (d) => streams[name].out.push(d.toString()))
process.stderr.on('data', (d) => streams[name].err.push(d.toString()))
}
const collectErrorStreams = (name, e) => {
e.stdout && streams[name].out.push(e.stdout)
e.stderr && streams[name].err.push(e.stderr)
}

// helper to output stream content on error
const printStreamsToConsole = async (name) => {
const std = streams[name]
if (std.out && std.out.length > 0) {
console.log(`stdout of ${name}\n${std.out.join('\n')}\n`)
}
if (std.err && std.err.length > 0) {
console.log(`stderr of ${name}\n${std.err.join('\n')}\n`)
}
}

// only run `vite build` when needed
if (isProd) {
const buildCommand = `${viteBin} build`
try {
const buildProcess = execa.command(buildCommand, {
cwd: root,
stdio: 'pipe'
})
collectStreams('build', buildProcess)
await buildProcess
} catch (e) {
console.error(`error while executing cli command "${buildCommand}":`, e)
collectErrorStreams('build', e)
await printStreamsToConsole('build')
throw e
}
}

// run `vite --port x` or `vite preview --port x` to start server
const viteServerArgs = ['--port', `${port}`, '--strict-port']
if (isProd) {
viteServerArgs.unshift('preview')
}
const serverCommand = `${viteBin} ${viteServerArgs.join(' ')}`
const serverProcess = execa.command(serverCommand, {
cwd: root,
stdio: 'pipe'
})
collectStreams('server', serverProcess)

// close server helper, send SIGTERM followed by SIGKILL if needed, give up after 3sec
const close = async () => {
if (serverProcess) {
const timeoutError = `server process still alive after 3s`
try {
killProcess(serverProcess)
await resolvedOrTimeout(serverProcess, 3000, timeoutError)
} catch (e) {
if (e === timeoutError || (!serverProcess.killed && !isWindows)) {
collectErrorStreams('server', e)
console.error(
`error while killing cli command "${serverCommand}":`,
e
)
await printStreamsToConsole('server')
}
}
}
}

try {
await startedOnPort(serverProcess, port, 3000)
return { close }
} catch (e) {
collectErrorStreams('server', e)
console.error(`error while executing cli command "${serverCommand}":`, e)
await printStreamsToConsole('server')
try {
await close()
} catch (e1) {
console.error(
`error while killing cli command after failed execute "${serverCommand}":`,
e1
)
}
}
}

// helper to validate that server was started on the correct port
async function startedOnPort(serverProcess, port, timeout) {
let checkPort
const startedPromise = new Promise((resolve, reject) => {
checkPort = (data) => {
const str = data.toString()
// hack, console output may contain color code gibberish
// skip gibberish between localhost: and port number
const match = str.match(/(http:\/\/localhost:)(?:.*)(\d{4})/)
if (match) {
const startedPort = parseInt(match[2], 10)
if (startedPort === port) {
resolve()
} else {
const msg = `server listens on port ${startedPort} instead of ${port}`
reject(msg)
}
}
}
serverProcess.stdout.on('data', checkPort)
})
return resolvedOrTimeout(
startedPromise,
timeout,
`failed to start within ${timeout}ms`
).finally(() => serverProcess.stdout.off('data', checkPort))
}

// helper function to kill process, uses taskkill on windows to ensure child process is killed too
function killProcess(serverProcess) {
if (isWindows) {
try {
execa.commandSync(`taskkill /pid ${serverProcess.pid} /T /F`)
} catch (e) {
console.error('failed to taskkill:', e)
}
} else {
serverProcess.kill('SIGTERM', { forceKillAfterTimeout: 2000 })
}
}

// helper function that rejects with errorMessage if promise isn't settled within ms
async function resolvedOrTimeout(promise, ms, errorMessage) {
let timer
return Promise.race([
promise,
new Promise((_, reject) => {
timer = setTimeout(() => reject(errorMessage), ms)
})
]).finally(() => {
clearTimeout(timer)
timer = null
})
}
3 changes: 3 additions & 0 deletions packages/playground/cli/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script type="module" src="./index.js"></script>

<div class="app">vite cli works!</div>
1 change: 1 addition & 0 deletions packages/playground/cli/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('vite cli works!')
11 changes: 11 additions & 0 deletions packages/playground/cli/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "test-cli",
"private": true,
"version": "0.0.0",
"scripts": {
"dev": "vite",
"build": "vite build",
"debug": "node --inspect-brk ../../vite/bin/vite",
"serve": "vite preview"
}
}
12 changes: 12 additions & 0 deletions packages/playground/cli/vite.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const { defineConfig } = require('vite')

module.exports = defineConfig({
server: {
host: 'localhost'
},
build: {
//speed up build
minify: false,
target: 'esnext'
}
})
61 changes: 43 additions & 18 deletions packages/playground/css/postcss-caching/css.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,51 @@ import path from 'path'

test('postcss config', async () => {
const port = 5005
const startServer = async (root) => {
const server = await createServer({
root,
logLevel: 'silent',
server: {
port,
strictPort: true
},
build: {
// skip transpilation during tests to make it faster
target: 'esnext'
}
})
await server.listen()
return server
}
const blueAppDir = path.join(__dirname, 'blue-app')
const greenAppDir = path.join(__dirname, 'green-app')
let blueApp
let greenApp
try {
blueApp = await startServer(blueAppDir)

process.chdir(blueAppDir)
const blueApp = await createServer()
await blueApp.listen(port)
await page.goto(`http://localhost:${port}`)
const blueA = await page.$('.postcss-a')
expect(await getColor(blueA)).toBe('blue')
const blueB = await page.$('.postcss-b')
expect(await getColor(blueB)).toBe('black')
await blueApp.close()
await page.goto(`http://localhost:${port}`)
const blueA = await page.$('.postcss-a')
expect(await getColor(blueA)).toBe('blue')
const blueB = await page.$('.postcss-b')
expect(await getColor(blueB)).toBe('black')
await blueApp.close()
blueApp = null

process.chdir(greenAppDir)
const greenApp = await createServer()
await greenApp.listen(port)
await page.goto(`http://localhost:${port}`)
const greenA = await page.$('.postcss-a')
expect(await getColor(greenA)).toBe('black')
const greenB = await page.$('.postcss-b')
expect(await getColor(greenB)).toBe('green')
await greenApp.close()
greenApp = await startServer(greenAppDir)
await page.goto(`http://localhost:${port}`)
const greenA = await page.$('.postcss-a')
expect(await getColor(greenA)).toBe('black')
const greenB = await page.$('.postcss-b')
expect(await getColor(greenB)).toBe('green')
await greenApp.close()
greenApp = null
} finally {
if (blueApp) {
await blueApp.close()
}
if (greenApp) {
await greenApp.close()
}
}
})
1 change: 1 addition & 0 deletions packages/playground/testUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export const isBuild = !!process.env.VITE_TEST_BUILD
const testPath = expect.getState().testPath
const testName = slash(testPath).match(/playground\/([\w-]+)\//)?.[1]
export const testDir = path.resolve(__dirname, '../../packages/temp', testName)
export const workspaceRoot = path.resolve(__dirname, '../../')

const hexToNameMap: Record<string, string> = {}
Object.keys(colors).forEach((color) => {
Expand Down
13 changes: 5 additions & 8 deletions packages/vite/src/node/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,19 +106,16 @@ cli
throw new Error('HTTP server not available')
}

await server.listen()

printHttpServerUrls(server.httpServer, server.config, options)

// @ts-ignore
if (global.__vite_start_time) {
info(
chalk.cyan(
// @ts-ignore
performance.now() - global.__vite_start_time
)
)
// @ts-ignore
const startupDuration = performance.now() - global.__vite_start_time
info(`\n ${chalk.cyan(`ready in ${Math.ceil(startupDuration)}ms.`)}\n`)
}

await server.listen()
} catch (e) {
createLogger(options.logLevel).error(
chalk.red(`error when starting dev server:\n${e.stack}`),
Expand Down
4 changes: 2 additions & 2 deletions packages/vite/src/node/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export function printHttpServerUrls(
}
}

export function printServerUrls(
function printServerUrls(
hostname: Hostname,
protocol: string,
port: number,
Expand All @@ -176,7 +176,7 @@ export function printServerUrls(
} else {
Object.values(os.networkInterfaces())
.flatMap((nInterface) => nInterface ?? [])
.filter((detail) => detail.family === 'IPv4')
.filter((detail) => detail && detail.address && detail.family === 'IPv4')
.map((detail) => {
const type = detail.address.includes('127.0.0.1')
? 'Local: '
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.