Skip to content

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

Merged
timmywil merged 2 commits intojquery:mainfrom
mgol:max-dots-per-line
Apr 1, 2026
Merged

fix(reporter): don't print all dots in one line#31
timmywil merged 2 commits intojquery:mainfrom
mgol:max-dots-per-line

Conversation

@mgol
Copy link
Copy Markdown
Member

@mgol mgol commented Mar 9, 2026

CI systems like GitHub Actions only print a line when a newline character is printed. Add a newline character after each 100 dots to flush output mid-test.

Also, fix the lockfile (without it, CI doesn't pass).

Fixes gh-30

CI systems like GitHub Actions only print a line when a newline character
is printed. Add a newline character after each 100 dots to flush output
mid-test.

Also, fix the lockfile (without it, CI doesn't pass).

Fixes jquerygh-30
@mgol mgol force-pushed the max-dots-per-line branch from a2d76a3 to 643f369 Compare March 9, 2026 21:22
@mgol
Copy link
Copy Markdown
Member Author

mgol commented Mar 9, 2026

The Test jQuery Integration runs

Before

Screenshot 2026-03-09 at 22 25 01

After

Screenshot 2026-03-09 at 22 25 25

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves the test runner’s progress reporter output so CI logs flush periodically during long pass-only runs, and updates package-lock.json so dependency installation succeeds in CI.

Changes:

  • Add a dot writer helper that inserts a newline after every 100 passed tests to encourage CI log flushing.
  • Regenerate/fix package-lock.json entries (including updating @simple-libs/stream-utils to 1.2.0 and removing various peer markers).

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
reporter.js Adds dot batching/newline flushing logic for CI visibility.
package-lock.json Updates lockfile metadata/entries to restore passing CI installs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread reporter.js
Comment on lines +19 to +29
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( "." );
}
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

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

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

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

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

@timmywil timmywil merged commit fda24e5 into jquery:main Apr 1, 2026
8 checks passed
@mgol mgol deleted the max-dots-per-line branch April 15, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dots indicating test runs should have max length

3 participants