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.

Sign up for GitHub

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: don't toggle cli cursor on non-TTY #7336

Merged
merged 2 commits into from
Feb 3, 2025
Merged

fix: don't toggle cli cursor on non-TTY #7336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f80a646
fix: don't toggle cli cursor on non-TTY
AriPerkkio Jan 22, 2025
95810dd
Merge branch 'main' into fix/dont-toggle-cursor-ci
AriPerkkio Jan 24, 2025
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
6 changes: 0 additions & 6 deletions packages/vitest/src/node/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,6 @@ export class Vitest {
}

async collect(filters?: string[]): Promise<TestRunResult> {
this._onClose = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these removals also the part of the fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, currently all onClose listeners were completely ignored for some reason.

vitest/packages/vitest/src/node/core.ts

Lines 120 to 124 in 926ca95

constructor(
public readonly mode: VitestRunMode,
options: VitestOptions = {},
) {
this.logger = new Logger(this, options.stdout, options.stderr)

vitest/packages/vitest/src/node/logger.ts

Lines 320 to 325 in 926ca95

this.ctx.onClose(() => {
process.off('SIGINT', onExit)
process.off('SIGTERM', onExit)
process.off('exit', onExit)
cleanup()
})

vitest/packages/vitest/src/node/core.ts

Lines 545 to 546 in 926ca95

async start(filters?: string[]): Promise<TestRunResult> {
this._onClose = []

This prevented cli cursor clearing. The added test case was failing without this.


const files = await this.specifications.getRelevantTestSpecifications(filters)

// if run with --changed, don't exit if no tests are found
Expand Down Expand Up @@ -543,8 +541,6 @@ export class Vitest {
* @param filters String filters to match the test files
*/
async start(filters?: string[]): Promise<TestRunResult> {
this._onClose = []

try {
await this.initCoverageProvider()
await this.coverageProvider?.clean(this.config.coverage.clean)
Expand Down Expand Up @@ -602,8 +598,6 @@ export class Vitest {
* If the `--watch` flag is provided, Vitest will still run changed tests even if this method was not called.
*/
async init(): Promise<void> {
this._onClose = []

try {
await this.initCoverageProvider()
await this.coverageProvider?.clean(this.config.coverage.clean)
Expand Down
9 changes: 7 additions & 2 deletions packages/vitest/src/node/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ export class Logger {
this.addCleanupListeners()
this.registerUnhandledRejection()

;(this.outputStream as Writable).write(HIDE_CURSOR)
if ((this.outputStream as typeof process.stdout).isTTY) {
(this.outputStream as Writable).write(HIDE_CURSOR)
}
}

log(...args: any[]) {
Expand Down Expand Up @@ -298,7 +300,10 @@ export class Logger {
private addCleanupListeners() {
const cleanup = () => {
this.cleanupListeners.forEach(fn => fn())
;(this.outputStream as Writable).write(SHOW_CURSOR)

if ((this.outputStream as typeof process.stdout).isTTY) {
(this.outputStream as Writable).write(SHOW_CURSOR)
}
}

const onExit = (signal?: string | number, exitCode?: number) => {
Expand Down
26 changes: 26 additions & 0 deletions test/reporters/tests/logger.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { expect, test } from 'vitest'
import { runVitest } from '../../test-utils'

test('cursor is hidden during test run in TTY', async () => {
const { stdout } = await runVitest({
include: ['b1.test.ts'],
root: 'fixtures/default',
reporters: 'none',
watch: false,
}, undefined, undefined, undefined, { tty: true, preserveAnsi: true })

expect(stdout).toContain('\x1B[?25l')
expect(stdout).toContain('\x1B[?25h')
})

test('cursor is not hidden during test run in non-TTY', async () => {
const { stdout } = await runVitest({
include: ['b1.test.ts'],
root: 'fixtures/default',
reporters: 'none',
watch: false,
}, undefined, undefined, undefined, { preserveAnsi: true })

expect(stdout).not.toContain('\x1B[?25l')
expect(stdout).not.toContain('\x1B[?25h')
})
6 changes: 6 additions & 0 deletions test/test-utils/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface VitestRunnerCLIOptions {
std?: 'inherit'
fails?: boolean
preserveAnsi?: boolean
tty?: boolean
}

export async function runVitest(
Expand All @@ -47,6 +48,11 @@ export async function runVitest(
callback()
},
})

if (runnerOptions?.tty) {
(stdout as typeof process.stdout).isTTY = true
}

const stderr = new Writable({
write(chunk, __, callback) {
if (runnerOptions.std === 'inherit') {
Expand Down
Loading