Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyze source & test dependencies to figure out which tests to run (first) #1986

Open
NathanielHill opened this issue Nov 21, 2018 · 12 comments
Labels
💵 Funded on Issuehunt This issue has been funded on Issuehunt enhancement new functionality help wanted

Comments

@NathanielHill
Copy link

NathanielHill commented Nov 21, 2018

Issuehunt badges

Just trying out AVA instead of Jest in a new monorepo project. I am configuring testing to be run as a pre-commit hook using husky and lint-staged.

I've run into a problem getting the appropriate tests run by ava during a commit. Previously, I would use lint-staged which would pass the changed files as arguments to jest and using the --findRelatedTests flag, jest would run only the tests that covered those files.

Trying to do something similar with ava has left me at a dead-end. If I try to use lint-staged to run ava, it passes a list of changed files and ava tries to run them as tests which fails. The alternative is simply to run the entire test suite, which will become inefficient as the project grows.

Any thing I'm missing, or thoughts on how to run appropriate ava tests as part of a pre-commit process?

There is a $80.00 open bounty on this issue. Add more on Issuehunt.

@momocow
Copy link

momocow commented Nov 21, 2018

I have come up with 2 ideas worthy to have a try:

  1. Filter those files from lint-staged whose filename is ended with .test.js.
  2. How about trying --match flag provided by ava command, which can filter the title of tests?
    (FYI: https://github.com/avajs/ava/blob/master/docs/05-command-line.md#running-tests-with-matching-titles)

Don't blame me if not working ;)
I just happened to read this issue and thought that these ideas might help.

@havenchyk
Copy link

Seems to be a feature request, what do you think @novemberborn?

@NathanielHill
Copy link
Author

@momocow Thanks for the thoughts, I don't think either of those really work.

  1. Using lint-staged I can specifically only apply actions to test files, but this means these actions are run when the test files themselves change, not the files the tests apply to. Not really what I need.
  2. This might work, if I put the filename of the tests in each test. However, lint-staged is passing complete pathnames, so this would require an additional step to strip the paths down to a relative path. Also, seems terribly fragile - for example, how does this work for integration tests where I may be importing components or modules from multiple files?

@havenchyk I'm new to ava, but I guess this is a feature request then. Seems like a pretty critical one IMHO, because the workflow has always been so straightforward for me and just makes sense. Running related tests before every code commit is great DX, and good practice.

@NathanielHill
Copy link
Author

Doesn't seem too complicated. If given a list of test files and a list of staged files, all that needs to be done is determine which if any test files import from the staged file list (passed as command line arguments). My vote would be to keep/use the files config for the former list.

@novemberborn
Copy link
Member

Yea AVA doesn't do this. Watch mode merely notes which source files are depended on my test files, and uses that to re-run specific test files.

It should be possible to do this outside of AVA by analyzing the dependency tree to select the test files to run. It'd be neat if AVA could do the same to prioritize which tests to run.

Would be great if somebody could spec out how this should work and we can take it from there.

@novemberborn novemberborn changed the title How to run related tests? Analyze source & test dependencies to figure out which tests to run (first) Nov 25, 2018
@sholladay
Copy link

sholladay commented Dec 1, 2018

Until there is a robust system to analyze the actual test and source files to figure out what to run, you could get 90% of the way there by using sed or awk in your pre-commit script. Conceptually, what you want to do is map the output of lint-staged and append .test.js to each of the filepaths that it outputs. AVA will then run those tests. So if you change foo.js, it will run foo.test.js.

It's not perfect, because foo.test.js may not necessarily be the only test that imports (or indirectly relies upon) foo.js. But it's a good start, especially if you can stick to a 1:1 relationship between source files and tests.

@AgentBurgundy
Copy link

Until there is a robust system to analyze the actual test and source files to figure out what to run, you could get 90% of the way there by using sed or awk in your pre-commit script. Conceptually, what you want to do is map the output of lint-staged and append .test.js to each of the filepths that it outputs. AVA will then run those tests. So if you change foo.js, it will run foo.test.js.

It's not perfect, because foo.test.js may not necessarily be the only test that imports (or indirectly relies upon) foo.js. But it's a good start, especially if you can stick to a 1:1 relationship between source files and tests.

I'm new to this repository, so my solution is a little rough haha, but after reading what @sholladay said, I figured it would be simple to implement. I got it working by adding a flag to the cli

prioritize: {
    type: 'string',
    default: []
}

Which I later use in cli.js where the test files are normally assigned to a constant.\

let files = '';
if (cli.flags.prioritize.length > 0) {
	console.log(`You're prioritizing ${cli.flags.prioritize}`);
	files = cli.flags.prioritize.split(' ').forEach(file => {
		let temp = file;
		
		file = file.replace('.js', '.test.js');
		
		console.log(`${temp} is now ${file}`);
	});
} else {
	files = cli.input.length > 0 ? cli.input : arrify(conf.files);
}

Of course there are some issues here, we would probably want to pull the desired test suffix out of the ava config file for one.

@novemberborn
Copy link
Member

@AgentBurgundy you can already pass specific file paths to AVA, so you could do this without changing AVA itself.

@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 10, 2019
@IssueHuntBot
Copy link

@IssueHunt has funded $80.00 to this issue.


@novemberborn
Copy link
Member

FWIW, AVA now treats the file paths you provide as a filter over the tests it would execute normally. Which means you can pass staged files.

AVA itself now supports just JavaScript, with Babel and TypeScript support implemented in separate packages. Parsing the source tree to build a priority list is possible but difficult. Still, it'd be cool to have and doesn't change our other internals.

@rrichardson
Copy link
Contributor

rrichardson commented Dec 14, 2021

This is complicated a bit by my use case, which is a yarn workspace monorepo with dozens of modules. Tests live in the test directory under their respective module.
I'd need to find all staged files (actually last commit in my case, for pre-push) then identify the module whence they came, then pass in <module>/test/**/*.ts

It makes me think that what we need is a 3rd party module that does the above (if it doesn't already exist)
Something that will find dirs or files from stage/commits that meet criteria.

It also makes me think that what jest was doing was magic, because it did what I just described pretty well. (https://jestjs.io/docs/cli#--lastcommit)

@Kurt-von-Laven
Copy link

However, lint-staged is passing complete pathnames, so this would require an additional step to strip the paths down to a relative path.

@NathanielHill, it doesn’t solve the overall problem, but this step can be achieved via lint-staged --relative.

@rrichardson, FWIW, this is the crux of Jest’s solution, as explained here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💵 Funded on Issuehunt This issue has been funded on Issuehunt enhancement new functionality help wanted
Projects
None yet
Development

No branches or pull requests

9 participants