Skip to content

fix(reporter): don't print all dots in one line #31

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

Merged
timmywil merged 2 commits into from
Apr 1, 2026
Merged

fix(reporter): don't print all dots in one line #31

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
643f369
fix(reporter): don't print all dots in one line
mgol Mar 9, 2026
da44586
fixup! fix(reporter): don't print all dots in one line
mgol Mar 10, 2026
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
38 changes: 3 additions & 35 deletions package-lock.json
View file
Open in desktop

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

22 changes: 20 additions & 2 deletions reporter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import chalk from "chalk";
import { prettyMs } from "./lib/prettyMs.js";
import * as Diff from "diff";

const MAX_DOTS_PER_LINE = 100;

function serializeForDiff( value ) {

// Use naive serialization for everything except types with confusable values
Expand All @@ -14,11 +16,23 @@ function serializeForDiff( value ) {
return `${ value }`;
}

let currDots = 0;
function writeDot() {
if ( currDots >= MAX_DOTS_PER_LINE ) {

// Write a newline character occasionally to force a CI output flush.
process.stdout.write( "\n" );
currDots = 0;
}
currDots++;
process.stdout.write( "." );
}
Comment on lines +19 to +29
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

currDots is module-global state but it’s never reset when non-dot output is printed (e.g., failing tests call console.log("\n\n..." ), and callers like run.js also print blank lines/warnings). This can desync the dot counter from the actual line state and lead to extra/blank newlines being inserted by writeDot() (e.g., after exactly 100 dots and then a failure message). Consider resetting currDots = 0 whenever the reporter emits non-dot output (before console.log/console.error in this module), or exposing a small reset helper that call sites can use before printing their own output.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@mgol mgol Mar 9, 2026

Choose a reason for hiding this comment

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

I didn't want to complicate other logging, so this is a tradeoff I made on purpose. We'd have to create a wrapper logging function & use it instead of console. Doable, but I'm not sure it's worth it.

Copy link
Copy Markdown
Member

@timmywil timmywil Mar 10, 2026

Choose a reason for hiding this comment

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

It could perhaps reset the counter on reportEnd or reportError.

Copy link
Copy Markdown
Member Author

@mgol mgol Mar 10, 2026

Choose a reason for hiding this comment

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

That won't cover all cases, like QUnit printing to the console, but these two are common enough that maybe we could handle them.

Copy link
Copy Markdown
Member Author

@mgol mgol Mar 10, 2026

Choose a reason for hiding this comment

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

Added. Not the greatest improvement if you ask me as we still get shorter lines after all these "Chrome acknowledged" etc. messages, but these two were in the same file, so it was trivial to add.

Screenshot 2026-03-10 at 22 17 12


export function reportTest( test, { fullBrowser, id } ) {
if ( test.status === "passed" ) {

// Write to console without newlines
process.stdout.write( "." );
// Write to console
writeDot();
return;
}

Expand Down Expand Up @@ -114,6 +128,8 @@ export function reportTest( test, { fullBrowser, id } ) {
}

export function reportError( error ) {
currDots = 0;

const title = `${ error.name || "Error" }: ${ error.message }`;
let message = chalk.red( title );

Expand All @@ -125,6 +141,8 @@ export function reportError( error ) {
}

export function reportEnd( result, { descriptiveUrl, fullBrowser, id } ) {
currDots = 0;

console.log(
`\n\nTests finished in ${ prettyMs( result.runtime ) } ` +
`at ${ chalk.yellow( descriptiveUrl ) } ` +
Expand Down