chore: refactor config tests

[david-luna]

Description

Move config tests to a dedicated folder and also tried to simplify them. The tests rely on an array of config definitions with the following shape:

{
  name: ' nameOfTheConfig',
  defaultValue: 'value',    // defined if has a default
  envVar: 'ENV_VAR_NAME',    // defined if option can be set via ENV
  types: [ 'boolean' ],    // defined if there is a normalizer method 
  centralConfig: ' central_config_name',    // defined if it has a central config
}

Checklist

• Implement code
• Add tests
• Update TypeScript typings
• Update documentation
• Add CHANGELOG.asciidoc entry
• Commit message follows commit guidelines

[trentm]

nit: My preference would be that utility test functions do not call the t.end() themselves. While I know this means that the tests below need to write t.end() more times, it does mean that it is a bit easier to read a given test block:

test(..., t => {
    ...
})

and see if it explicitly calls t.end().

[trentm]

nit: Perhaps calling this function assertEnvVar or similar, which is more of a current naming pattern:

% rg 'function test' test | wc -l
       5
% rg 'function assert' test | wc -l
      50

[trentm]

It would be nice if this was data-driven eventually. I.e. if the set of config vars tested here was from an export of lib/config/schema.js. Is that the eventual intention?

I see that is done for NUM tests below.

[david-luna]

Really nice indeed. For now this 1st attempt is quite simple since the test only checks one option in isolation and both captureStackTraces and asyncHooks have dependencies. We could add a deps property on the schema entries, this way the filter function is enough

options definition would become

{
  name: ' nameOfTheConfig',
  defaultValue: 'value',  // defined if has a default
  envVar: 'ENV_VAR_NAME',  // defined if option can be set via ENV
  deps: ['nameOfAnotherConfig'],  // defined if there are dependencies on other configs
  types: [ 'boolean' ],  // defined if there is a normalizer method 
  centralConfig: ' central_config_name',  // defined if it has a central config
}

and the filter

const BOOL_OPTS_WITH_ENV_AND_NO_DEPS = CONFIG_DEFINITIONS.filter(function (def) {
  return ('envVar' in def) && !def.deps && hasType(def, 'boolean')
})

[david-luna]

I think it might be even better for discussions and improvements to have CONFIG_SCHEMA written on a direct export instead of having in a procedural way. I'll keep the generator code just in case we need to generate it again

[trentm]

Do you intend to define the CONFIG_SCHEMA in "./lib/config/..." files eventually? I'm a little confused why we would have an intermediate schema object just defined in the tests.

[david-luna]

Indeed that's the intent although it may have some slight changes at the end. The place for CONFIG_SCHEMA shall be ./lib/config/schema.js

I think this tests will give us more confidence to move to the new schema before doing the refactor (in which we will remove this file and use the actual schema)