Support defining test reporter using environment variable

[remcohaszing]

What is the problem this feature will solve?

#45712 added support for custom test reporters, which is great!

It’s still a but cumbersome having to specify the --test-reporter flag for every test run.

What is the feature you are proposing to solve the problem?

Use an environment variable NODE_TEST_REPORTER. If this is set, this will override the default tap runner. The command line argument will still take precedence. If this is specified, it will entirely ignore the NODE_TEST_REPORTER environment variable.

# Uses the tap formatter
node --test

# Uses the spec formatter
NODE_TEST_REPORTER=spec node --test

# Uses the tap formatter
NODE_TEST_REPORTER=spec node --test --test-reporter tap

This allows users to define their preferred formatter in their environment, e.g. in ~/.bashrc or ~/.zshrc

What alternatives have you considered?

It could also be supported in NODE_OPTIONS. However, people use this for different purposes too.

[cjihrig]

It's unlikely that we will add an environment variable for this. The general consensus is for things like that to be done via NODE_OPTIONS (otherwise we would end up supporting CLI flags and environment variables for every option supported by Node).

[MoLow]

@cjihrig --test-reporter is not allowed in NODE_OPTIONS and neither are any other test runner flags. perhaps we should change that

[cjihrig]

@MoLow sounds fine to me. The main reason for not doing so up to this point is that we'll need to parse the NODE_OPTIONS environment variable in the CLI runner when spawning child processes to make sure this logic is preserved.

[SRHerzog]

Re: the last couple of comments, I can dig into this if no one else is busy with it.

[remcohaszing]

The problem I see putting this in NODE_OPTIONS, is that a user may specify multiple --test-reporter and --test-reporter-destination flags, and order matters. Should these flags, when specified in NODE_OPTIONS, be discarded if the user passes these flags explicitly? Should they be appended?

Another solution could be to introduce a Node user configuration file ($XDG_CONFIG_HOME/node/config.json), but thats quite a big deviation from current approaches for options. This could contain other options, e.g. to configure the repl.

{
  "repl": {
    "ignoreUndefined": true
  },
  "test-reporter": [
    "spec",
    ["tap", "output.tap"]
  ]
}

[SRHerzog]

I think the intuitive behavior would be to ignore the values in NODE_OPTIONS when flags are passed via the CLI.

[cjihrig]

NODE_OPTIONS has documented behavior for such edge cases: https://nodejs.org/api/cli.html#node_optionsoptions

[SRHerzog]

This mostly works but will break on a --test-reporter or --test-reporter-destination value with a space in it, because I just split the NODE_OPTIONS string rather than properly parsing out quoted values. I can keep going to fully parse NODE_OPTIONS, assuming we want to make sure that values with spaces are supported properly.

[simoneb]

@SRHerzog why do we need to parse them again? Surely there must be a place in the codebase where the parsing is already done for us, and there are other options which support a space between the option name and the value, e.g. --require.

[SRHerzog]

NODE_OPTIONS is parsed in the C++ layer and merged with the CLI arguments, along with defaults where appropriate. It's read by the JavaScript layer as a Map after that merge is complete. There isn't a binding for the JavaScript runtime to retrieve NODE_OPTIONS in tokenized form or extract the NODE_OPTIONS values from the overall options map, so we would have to add that kind of functionality to node_options.cc if we want to avoid parsing the whole string again.

[simoneb]

Gotcha, I'm not familiar enough with the codebase to know when things happen exactly, I was just assuming that they must be happening somewhere. I assume that that when happens to late for us to make any use of it in this context. I wonder, aren't there other NODE_OPTIONS entries which the test runner is already making use of? How are they parsed?

[SRHerzog]

The only option specific to the test runner that can currently be passed through NODE_OPTIONS is --test-only. That one is passed through to child processes if it's set from the CLI and not filtered out like --test-reporter and --test-reporter-destination.