Mocha.utils.lookupFiles is no longer available

[auroq]

The commit linked below migrated lookupFiles from utils to cli. This caused it to be removed from Mocha.utils.lookupFiles, and it appears that it is no longer accessible anywhere.

This was still exposed as of 8.0.1 but is no longer available as of 8.1.1.

Additionally, Mocha.utils.lookupFiles does still appear in the current version of @types/mocha.

We are using this when calling mocha.run programatically.

This is the call we are making to lookupFiles

const testFiles = Mocha.utils
    .lookupFiles('**/*spec.[tj]s')
    .filter((file) => !file.includes('node_modules'));

40f951b#diff-50e3aa130a4f97a42ee2cf111c7b1d9d

[juergba]

Yes, it was moved to lib/cli/lookup-files.js in order to exclude it from our browser build. lookupFiles still is a public function and should be accessable by requiring it.

@types/mocha: is not maintained by us. We don't have resources to offer our own version of TypeScript type definitions.

[auroq]

The only place we have found this exposed is in mocha/lib/cli/lookup-files. Are we missing it somewhere else? If not, could we export it somewhere closer to the mocha index for greater visibility?

const lookupFiles = require('mocha/lib/cli/lookup-files');

[juergba]

Are we missing it somewhere else?

No, I don't think so, must be correct.

I'm not convinced of exporting it somewhere else for greater visibilty. We use this function only in CLI context and need to keep it out of our browser bundle.

The API documentation is incomplete, I can't find lookupFiles anymore. Some JSDoc tags must be wrong or missing (@module ?). So I admit it's difficult to re-find lookupFiles at the present.

[auroq]

That makes sense that it is generally only used in the CLI context. However, I feel that a like lookupFiles is key to being able to call mocha programmatically as we are doing.

I know running a testing framework isn't the primary method of using them (In our case our tests must be run inside of serverless infrastructure), so if mocha isn't intended to be run programmatically (or doesn't officially support that) I understand, and it would be great to see a line about that in the documentation.

On the other hand, if calling mocha programmatically is supported, then this is my request to expose lookupFiles elsewhere for this use case or at least update documentation to make it visible in its new location.

Regardless, thank you so much for maintaining this library. I truly appreciate your swift and candid responses.

[MoonSupport]

I agree with @auroq's opinion.
If isn't weird to import lookupFiles at cli/lookup-files, it seems good to write it in a document without any changes.
lookupFiles still looks useful for someone to call mocha programmatically. :)

[boneskull]

Either way, since lookupFiles was a public API and it moved, it was a breaking change that we didn't catch. Apologies!

1. We can't move it back into lib/utils, because browser
2. We can't move it into lib/mocha (which is what you get when you require('mocha')), because browser
3. We should make sure it shows up in the API documentation

The Node.js-specific public APIs we have are very few:

1. lookupFiles() in lib/cli/lookup-files
2. loadRc() in lib/cli/options
3. loadPkgRc() in lib/cli/options
4. loadOptions() in lib/cli/options

For the sake of ergonomics, I think it makes sense to alias these in lib/cli/index.js, like so:

const {lookupFiles, loadRc, loadPkgRc, loadOptions} = require('mocha/lib/cli');

We can make this a little easier to use with subpath exports, (e.g., aliasing mocha/lib/cli/index.js to mocha/cli) but I'm not really comfortable with this issue forcing a decision on how to configure our subpath exports.

[boneskull]

See #4419 for changes regarding this

[Munter]

Does lookupfiles really need to be a public API? I know we accidentally made a breaking change here, and that's very unfortunate. But really all that lookupfiles is, is a glob function. There are myriads of implementations of glob to take off the shelf. Why does mocha need to maintain another one as a public API?

[nicojs]

I think a proper public API for running mocha programmatically is what's missing here.

Related issues:
#3882
#4419

Either way, since lookupFiles was a public API and it moved

It was a public API in the sense that it was documented in API docs. Personally, I don't like a big API surface, since it's hard to maintain (hence this issue). It's better to have a smaller one (as discussed in #3882) and then have proper integration tests for them.

Maybe it's good to combine these thoughts into a new issue? I'd be happy to do so. I'm a consumer of the API myself and thus share the "pain".

I would like to focus on that whenever I'm done with my current priority (sorry). Hopefully within a month or so.

[boneskull]

Does lookupfiles really need to be a public API? I know we accidentally made a breaking change here, and that's very unfortunate. But really all that lookupfiles is, is a glob function. There are myriads of implementations of glob to take off the shelf. Why does mocha need to maintain another one as a public API?

I mean lookupFiles supports globs but also allows the caller to not use a glob; instead, you can provide a directory, file extension(s), and/or a "recursive" flag. The only reason it does this is because that's the behavior Mocha's CLI supports, and Mocha's CLI supports that behavior because Mocha's CLI supports that behavior. 😄

I would like to know from @auroq and/or @MoonSupport if they need lookupFiles()'s specific behavior, or if they could swap it out for a call to, say, glob()?

Here's a naive search for usages of lookupFiles().

That said, personally, I can tolerate absorbing the maintenance burden of lookupFiles as a public Mocha API if the alternative is breaking users. lookupFiles as a public API is of dubious necessity, but lots of things in Mocha are of dubious necessity... yet there they are.

I think what we need to do before considering merging #4419 is to un-break Mocha v8.1.0 with another patch release which restores the original path to lookupFiles (in util), perhaps with a soft-deprecation notice. I will try to figure out how without befouling the bundle.

[boneskull]

actually maybe it makes more sense to just add that to #4419, so I can reference the new location.