🐛 Bug: when using exports interface for tests, there's no file on suites objects

[shadowusr]

Bug Report Checklist

• I have read and agree to Mocha's Code of Conduct and Contributing Guidelines
• I have searched for related issues and issues with the faq label, but none matched my issue.
• I have '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, my usage of Mocha, or Mocha itself.
• I want to provide a PR to resolve this

Expected

You can access the file field on the suite object regardless of the interface you use.

Actual

file is undefined for exports mocha interface, but works correctly for bdd interface.

Minimal, Reproducible Example

1. Use Mocha API with exports interface:

const mocha = new Mocha({ui: "exports"});

2. Subscribe to events to receive suite object and load files:

const eventBus = MochaEventBus.create(mocha.suite);
eventBus.on(MochaEventBus.events.EVENT_SUITE_ADD_SUITE, (suite) => {
    console.log(suite.file); // will output undefined
});

await mocha.loadFilesAsync();

3. See that suite.file is undefined.

Versions

10.2.0, 10.4.0

Additional Info

We use mocha API in on of our tools internally and this use case is crucial for us. Otherwise, we'll need to implement dirty hacks to retrieve file field from test objects.

[JoshuaKGoldberg]

Thanks for filing!

Minimal, Reproducible Example

This is most of the way to an MRE but:

• It doesn't explain what MochaEventBus is
• It doesn't include what file(s) need to be on disk to populate suite.file

Could you please provide a way for us to try out your issue locally? I think I get it, but really need a local reproduction to be sure.

[shadowusr]

@JoshuaKGoldberg yeah sorry for not providing totally reproducible example.

Here it goes: https://stackblitz.com/edit/stackblitz-starters-pnnurz?file=mocha-bdd-demo.js

You can try running node mocha-bdd-demo.js vs node mocha-exports-demo.js. In the bdd case, you'll see file path on suite, but on the exports ui it will be undefined.

[shadowusr]

@JoshuaKGoldberg sorry to ping you again, but do we have any updates on this? Looks like a tiny issue to me with a ready PR, I would really appreciate if you could take a look at it.

[JoshuaKGoldberg]

No update just yet - we've all been pretty swamped on the maintainer team. It's in our queue. Sorry for not having anything more specific.

For context, in addition to this & other issues in triage, we've got some PR reviews to finalize (time boxed to the end of this month), as well as some administrative work going on behind-the-scenes still.

[JoshuaKGoldberg]

Ok! Sorry for the delay: yes, I think this makes sense. I'll throw this in front of @mochajs/maintenance-crew just in case I'm missing something, but 👍 from me.

[voxpelli]

Looks like all the other three ui options call this common.suite.create, which in turn call Suite.create:

mocha/lib/interfaces/common.js

Lines 132 to 135 in 24560c1

	create: function create(opts) {
	var suite = Suite.create(suites[0], opts.title);
	suite.pending = Boolean(opts.pending);
	suite.file = opts.file;

See eg. bdd.js:

mocha/lib/interfaces/bdd.js

Lines 41 to 47 in 24560c1

	context.describe = context.context = function (title, fn) {
	return common.suite.create({
	title,
	file,
	fn
	});
	};

But exports.js work different, it calls Suite.create directly in some cases and in other cases uses the Suite created as part of Mocha itself.

mocha/lib/interfaces/exports.js

Lines 22 to 59 in 24560c1

	module.exports = function (suite) {
	var suites = [suite];
	suite.on(Suite.constants.EVENT_FILE_REQUIRE, visit);
	function visit(obj, file) {
	var suite;
	for (var key in obj) {
	if (typeof obj[key] === 'function') {
	var fn = obj[key];
	switch (key) {
	case 'before':
	suites[0].beforeAll(fn);
	break;
	case 'after':
	suites[0].afterAll(fn);
	break;
	case 'beforeEach':
	suites[0].beforeEach(fn);
	break;
	case 'afterEach':
	suites[0].afterEach(fn);
	break;
	default:
	var test = new Test(key, fn);
	test.file = file;
	suites[0].addTest(test);
	}
	} else {
	suite = Suite.create(suites[0], key);
	suites.unshift(suite);
	visit(obj[key], file);
	suites.shift();
	}
	}
	}
	};

It does look like this was supposed to be fixed in 2015 through #1993, but it only added .file to tests.

I wonder if it would be breaking in any way to add suites[0].file = file before the for-loop here? That should ensure it's available everywhere.

mocha/lib/interfaces/exports.js

Lines 27 to 29 in 24560c1

	function visit(obj, file) {
	var suite;
	for (var key in obj) {

(I also wonder how often mocha is used with these non-bdd interfaces)

[voxpelli]

(I also wonder how often mocha is used with these non-bdd interfaces)

Answering myself somewhat: Summarizing mocharc files on sourcegraph gives that 75% specify bdd, 25% tdd and pretty much no one qunit or exports

That's probably not an exhaustive result though

[JoshuaKGoldberg]

I wonder if it would be breaking in any way to add suites[0].file = file before the for-loop here? That should ensure it's available everywhere.

IMO it should be fine as a purely additive change. We can treat it as a minor version rather than a patch if we really want to be careful.

[JoshuaKGoldberg]

Marking as accepting PRs for a semver-minor change. 🚀

[JoshuaKGoldberg]

By the way, sorry for the wait @shadowusr. I took a look at #5147 and it looks great to me - just one request for testing. Since it's been so long, I'd understand if you no longer have time for this and can add the test + merge it in a few weeks if you don't. Cheers! 🤎