test_runner: write timestamp as first line in watch mode - #59309
test_runner: write timestamp as first line in watch mode#59309OkunadeNaheem wants to merge 1 commit into
Conversation
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
| @@ -149,6 +149,13 @@ class TestsStream extends Readable { | |||
| this.#tryPush(null); | |||
| } | |||
|
|
|||
| // This event emitter logs a timestamp whenever a test restarts because of changes in any related files. | |||
There was a problem hiding this comment.
no need comment here?
Sorry, something went wrong.
There was a problem hiding this comment.
I have taken it out
Sorry, something went wrong.
| @@ -496,6 +496,12 @@ function watchFiles(testFiles, opts) { | |||
| } | |||
|
|
|||
| await runningSubtests.get(file); | |||
|
|
|||
| // Emit the 'test:restarted' event if a timeStamp reporter is available. | |||
There was a problem hiding this comment.
same
Sorry, something went wrong.
There was a problem hiding this comment.
This has been taken out also.
Sorry, something went wrong.
The goal: To always print a timestamp as the first line in watch mode to make test restarts clearer and logs easier to read. Continuation from https://github.com/nodejs/node/pull/57903/files solution, i simply created a new event 'timestamp' and then handled it. Also modified the test to directly test my changes from runner.js instead of test_streams.js file. Refs: nodejs#57206 Authors: https://github.com/JacopoPatroclo, https://github.com/OkunadeNaheem
pmarchini
left a comment
There was a problem hiding this comment.
In this PR we’re not introducing the timestamp as stated in the title.
What we should do instead is leverage the existing event so that the various reporters can print a restart message followed by the date.
The tests should then verify this behaviour!
Sorry, something went wrong.
|
|
||
| await runningSubtests.get(file); | ||
|
|
||
| opts.root.reporter[kEmitMessage]('test:watch:restarted'); |
There was a problem hiding this comment.
Hey @OkunadeNaheem, it looks like this branch was created from a very outdated main branch.
This addition was already made two months ago in this PR: #57903
Here's the line: https://github.com/nodejs/node/blob/main/lib/internal/test_runner/runner.js#L526
Sorry, something went wrong.
|
This pull request has been marked as stale due to 210 days of inactivity. |
Sorry, something went wrong.
|
This pull request has been marked as stale due to 90 days of inactivity. |
Sorry, something went wrong.
test_runner: write timestamp as first line in watch mode
The goal: To always print a timestamp as the first line in watch mode to make test restarts clearer and logs easier to read.
Continuation from https://github.com/nodejs/node/pull/57903/files solution, i simply created a new event 'timestamp' and then handled it. Also modified the test to directly test my changes from runner.js instead of test_streams.js file.
Refs: #57206