avajs / ava

Node.js test runner that lets you develop with confidence ๐Ÿš€
MIT License
20.74k stars 1.41k forks source link

Find ava.config.js files outside of project directory #2285

Closed novemberborn closed 2 years ago

novemberborn commented 4 years ago

To better work with monorepo setups it'd be useful if AVA would find ava.config.js files that are outside of the project directory for the package that AVA is running in (the directory containing the package.json file).

So when looking for ava.config.js, we'll keep looking in parent directories until we find an ava.config.js file. However, we'll stop looking once we find a directory that has a .git directory in it.

We'd support other VCS as well.

This is blocked by #2284.

novemberborn commented 4 years ago

This is a breaking change, since finding new configuration files could break how AVA is used. We'll have to land this as an experiment until it's ready to go out with the next major version.

Of course experiments are controlled through configuration, so let's see if we can add an --experiments flag on the CLI.

stavalfi commented 4 years ago

why your proposition is better than using a single ava.config.js in the root of the monorepo and running test from the root folder? in this way, tests may even run faster because ava would handle the concurrency and the parallelism while lerna/yarn-workspaces would.

What do you think?

novemberborn commented 4 years ago

why your proposition is better than using a single ava.config.js in the root of the monorepo and running test from the root folder? in this way, tests may even run faster because ava would handle the concurrency and the parallelism while lerna/yarn-workspaces would.

One problem is how to find the root. By default, AVA assumes that's the repository containing a package.json file.

This issue proposes we speculatively search parent directories, too, until we find a VCS root, hoping to encounter ava.config.* files along the way.

I think this would work well both for single-package-per-repository projects and multiple-package-per-repository projects.

shellscape commented 3 years ago

A very gentle nudge This continues to plague monorepos. As a massive fan of Ava I continue to push its use where and when I can, but this is always a pain point that I must argue through to gain acceptance. Rollup's plugins monorepo uses Ava and we've been forced to copy the same configuration into 28 separate package.json files. That's only going to continue to grow as the project adopts and creates more official plugins, and the pushback internally to switch to a tool that supports monorepos is starting to increase. I do hope this wasn't perceived as pressure, but the uphill struggle to use this, as a superior tool, is real when it comes to the increasingly popular monorepo setup.

stavalfi commented 3 years ago

A very gentle nudge This continues to plague monorepos. As a massive fan of Ava I continue to push its use where and when I can, but this is always a pain point that I must argue through to gain acceptance. Rollup's plugins monorepo uses Ava and we've been forced to copy the same configuration into 28 separate package.json files. That's only going to continue to grow as the project adopts and creates more official plugins, and the pushback internally to switch to a tool that supports monorepos is starting to increase. I do hope this wasn't perceived as pressure, but the uphill struggle to use this, as a superior tool, is real when it comes to the increasingly popular monorepo setup.

I think that my comment above can help you. run this command from the root: yarn ava to run all tests. yarn ava <specific package-path> to run all tests of specific package.

novemberborn commented 3 years ago

@shellscape

A very gentle nudge

Not sure who you're nudgingโ€ฆ there is an solution provided and the issue is labeled help wanted ๐Ÿ˜‰

we've been forced to copy the same configuration into 28 separate package.json files

You could use an ava.config.cjs file and at least share the same configuration that way. Until this issue is resolved.

shellscape commented 3 years ago

๐Ÿ˜‚

touche

@stavalfi while I'm sure that will help some folks, that's a package-manager-specific solution. So for those who don't use yarn (e.g. pnpm) we run into other issues with that proposal. Namely that Ava doesn't think it can find any files. Now, I'm 99.9% certain that's to do with pnpm and how it manages node_modules, so I didn't report that as an issue, as I don't think it's something Ava should be concerned about.

If I can get buy-in at my latest professional stop, I can probably get approval for time on this. If it doesn't go my way, not sure I'll be able to. But thanks for responding all the same.

novemberborn commented 3 years ago

@shellscape I had another look at the code to see how accurate the proposed solution still is. Since we now look for three different configuration files, accurately resolving them will take some disk access. And it'd be nice to parallelize that, which requires us to load configuration asynchronously.

https://github.com/avajs/ava/pull/2629 should get us there. And then we can make this an experimental feature to make default in AVA 4.

novemberborn commented 3 years ago

The resolution logic lives here:

https://github.com/avajs/ava/blob/a2f2614cc1e34fb9a7aa204b37b3b25559f203d9/lib/load-config.js#L202-L207

Adjustments should be made so it can find other candidate configuration files. Then, if they've opted into the nextGenConfig experiment, they can be selected. Note however that the current behavior must not change if the experiment is not enabled.

shellscape commented 3 years ago

@novemberborn apologies it took me a bit to circle back to this. the proposed solution looks solid, thank you for working on that. what are the next steps that are needed?

novemberborn commented 3 years ago

apologies it took me a bit to circle back to this.

No worries.

what are the next steps that are needed?

"Making it so", as it were ๐Ÿ˜„

The main branch is now developing for AVA 4 so the experiment notes are now irrelevant.

vjpr commented 3 years ago

Is there are reason why Config files must be located next to the package.json file? Could this be disabled?

I simply want to pass in a config file from my monorepo root. Otherwise, like shellscape said, I need an ava.config.cjs in every single package of the 100s of packages in my monorepo.

Is there any workaround?

novemberborn commented 3 years ago

This ticket is proposing we change that specifically to support monorepos, I want to get it into the v4 release it's one of the last few tasks left.

The challenge that Babel ran into is that folks would leave .babelrc files scattered across their filesystem that would kick in unexpectedly. So I'd like to limit the search for config files until say the repository root.

shellscape commented 3 years ago

Yeah that's a valid concern. Repo root limit is completely warranted.

@vjpr I use symlinks to the root/shared ava config. It's not perfect, but links are relatively easy to manage. For 100s of directories I'd recommend a script to CRUD the links.

lightmare commented 3 years ago

we'll stop looking once we find a directory that has a .git directory in it.

nit: .git can be a regular file.

novemberborn commented 3 years ago

nit: .git can be a regular file.

@lightmare in what sense?

lightmare commented 3 years ago

.git does not have to be a directory. It can be a text file with a path to the repository. Submodules and separate worktrees use this.

novemberborn commented 3 years ago

@lightmare thanks, that's good to know!