🐛 Bug: Some Settings Cannot Be Inherited (extends keyword)

[manuth]

Prerequisites

• Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
• 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
• Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Currently, mocha will load default settings from lib/mocharc.json from this package and only after doing so process the configuration using yargs.

This breaks yargs extends mechanism as the values predefined in said mocharc.json file cannot be overridden.

This means that following settings cannot be specified using the extends config:

• diff
• package
• reporter
• slow
• timeout
• ui

I assume that arrays still do work as, to my understanding, yargs will concatenate arrays taken from extends configs.

Steps to Reproduce

1. Create a project with mocha tests written using the tdd ui
2. In your project's root, create a file base.json with the following content:

   {
       "ui": "tdd"
   }

3. In your project's root, create a file called .mocharc.json:

   {
       "timeout": 10000
   }

4. Try to run your tdd tests by running the mocha command

Expected behavior:

mocha should be able to execute this test as the setting "ui": "tdd" is supposed to be inherited.

Actual behavior: [What actually happens]

mocha will instead show the following error message:

ReferenceError: suite is not defined

Reproduces how often: 100%

Versions

• The output of mocha --version and node_modules/.bin/mocha --version: 10.0.2
• The output of node --version: 20.0.0
• Your operating system
  • name and version: arch1
  • architecture (32 or 64-bit): amd64
• Your shell (e.g., bash, zsh, PowerShell, cmd): bash, PowerShell, cmd
• Your browser and version (if running browser tests):
• Any third-party Mocha-related modules (and their versions):
• Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version):

Additional Information

Notice that default values from this package's lib/mocharc.json file are applied to the current config file during the execution of import("mocha/lib/cli/options").loadOptions() in this piece of code:

mocha/lib/cli/options.js

Line 246 in 37deed2

args = parse(args._, mocharc, args, rcConfig || {}, pkgConfig || {});

Only after invoking loadOptions(), the extends setting is processed by yargs:

• First, loadOptions() is executed - in doing so, values from lib/mocharc.json are applied
  

  mocha/lib/cli/cli.js

  Line 47 in 37deed2

  var args = mochaArgs || loadOptions(argv);

• After that, features such as extends are processed by yargs:
  

  mocha/lib/cli/cli.js

  Line 77 in 37deed2

  .config(args)

Related to this question: #3916
There is also a PR addressing this issue: #4407
Furthermore, I created a PR myself: #4980

[JoshuaKGoldberg]

In your project's root, create a file base.json

This confuses me. Nothing in the .mocharc.json references extending base.json. So it makes sense that the values in base.json wouldn't be used.

Was the repro meant to have .mocharc.json look like this?

{
  "extends": "./base.json",
  "timeout": 10000
}

The PR makes sense at first glance, and I can reproduce the buggy behavior by adding in the missing "extends". But I just want to be sure. 🙂

[JoshuaKGoldberg]

👋 ping @manuth, is this still on your radar?

[JoshuaKGoldberg]

Closing this out as stale it's been more than a month. If anybody is seeing a bug like this and has a repro, please do post back. We'd be happy to take a look! ☕

[manuth]

Agreed - I assume we will be keeping track of this in #4980 anyways.