Example Custom Reporter Provides No Output at All When There's a Warning

[machineghost]

Version

20.3.0

Platform

Linux mycomputer 5.15.0-76-generic #83-Ubuntu SMP Thu Jun 15 19:16:32 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

test

What steps will reproduce the bug?

1. Create a copy of the sample custom test reporter (https://nodejs.org/api/test.html#custom-reporters)
2. Add --test-reporter=./test/testReporter.js to node test testFile.js
3. (Also add some other argument that causes a warning, eg. --experimental-loader ./loader.js)
4. Run tests

How often does it reproduce? Is there a required condition?

100% reproducible (for me at least)

What is the expected behavior? Why is that the expected behavior?

You see some (customized) test output.

What do you see instead?

You see nothing:

$ npm t

> test-app@0.1.0 test
> > node --experimental-loader ./loader.js  --test-reporter=./test/testReporter.js --test test.js

Additional information

First off, let me just say the built-in Node test runner is great (or at least a really great start), and I very much appreciate this feature! No more mocha/jest/ava/other annoying dependencies!

As for the issue, this is happening because the example test reporter has a switch/case statement, and it doesn't include a case for when event.type === 'test:stderr'. If you add:

default:
        console.log(event);

to the custom reporter's switch, you instead see:

(node:578700) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
{
  type: 'test:stderr',
  data: [Object: null prototype] {
    file: '/home/me/myproject/test/test.js',
    message: '(node:578712) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time\n'
  }
}

As someone new to custom reporters, it was very confusing to add the example one and get absolutely no output whatsoever. It seems to me that the sample test reporter should either:

A) If unexpected events (eg. warnings) are "errors", the sample test runner should treat them as such:

default:
    throw new Error('Unexpected event' + event.type)

B) if warnings and other events are expected (as it seems they should be), but they shouldn't output, that should instead be:

default:
  callback(null);

C) If warnings are expected, and should output (I think this is correct) it should be:

case 'test:warning':
  callback(null, 'test:warning');

However, if you go with C), it seems some sort of default (like A or B) should be included also, to handle any other unexpected events in some way (besides silently swallowing them).

[koshic]

@machineghost the root cause is that we must invoke callback in transform handler to get next available chunk. So, example code is wrong due to incorrect stream usage.

+1 to provide default case to the switch statement or change logic by introducing intermediate variable to store message and always invoke callback(null, message) at function end.