Skip to content

Conversation

@novemberborn
Copy link
Member

Fixes #593.

The watcher keeps track of exclusive tests. If all test files that contained exclusive tests need to be rerun it runs them without forcing exclusive mode. This means the exclusivity is determined by the tests themselves.

If a test file, containing exclusive tests, is not one of the files being rerun, it forces exclusive mode. This ensures only exclusive tests are run in the changed files, making .only sticky.

If all test files that contained exclusive tests are removed, sticky mode is disabled. The same happens if there are no more exclusive tests after a run.

A second argument can be passed to Api#run(). If true the tests will be run in exclusive mode, regardless of whether exclusive tests are detected. The Api now remits the 'stats' event from the forks.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @sotojuan, @ariporad and @spudly to be potential reviewers

@novemberborn
Copy link
Member Author

Note that if you edit a file without exclusive tests, AVA will report 0 tests passed. Let me know if this needs a more helpful message.

lib/watcher.js Outdated
});
};

Watcher.prototype.updateExlusivity = function (file, hasExclusive) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exlusivity -> exclusivity

@vadimdemedes
Copy link
Contributor

PR looks good @novemberborn, but I feel like we really have to come up with a replacement for exclusive-related names. hasExclusive, trackExclusivity, updateExclusivity sound very weird and unclear. At least to me. Let's hear what others think.

api.js Outdated
};

Api.prototype.run = function (files) {
Api.prototype.run = function (files, forceHasExclusive) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the second argument would be better as an optional options object with an option called forceHasExclusive.

http://ariya.ofilabs.com/2011/08/hall-of-api-shame-boolean-trap.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Opted for runOnlyExclusive which is the same as the option passed to the runner.

@novemberborn
Copy link
Member Author

I feel like we really have to come up with a replacement for exclusive-related names. hasExclusive, trackExclusivity, updateExclusivity sound very weird and unclear. At least to me. Let's hear what others think.

English is weird 😉

hasExclusive comes from TestCollection and made its way into the stats object and Api. "Exclusivity" here refers to the updating the exclusivity of test files. I've changed some of the variable names to be more accurate, hopefully it's clearer now.

@novemberborn
Copy link
Member Author

Pushed fixups for the feedback. Some of the changes were slightly more involved so would like feedback before squashing 😄

@sindresorhus
Copy link
Member

Note that if you edit a file without exclusive tests, AVA will report 0 tests passed. Let me know if this needs a more helpful message.

Can you show an example on when this would happen exactly? More helpful messages are always welcome.

@novemberborn
Copy link
Member Author

Can you show an example on when this would happen exactly? More helpful messages are always welcome.

Imagine you have two test files: test/foo.js and test/bar.js. foo.js contains a .only(). When you start the watcher it only runs that one test from foo.js. Now you edit bar.js. It runs the tests from bar.js in exclusive mode, but there are no such tests. It outputs 0 tests passed.

Maybe it should report 0 exclusive tests found, but that would entail modifying the various reporters.

It'd also help if I added debug output so you can see the tests are being run in exclusive mode.

@sindresorhus
Copy link
Member

Maybe it should report 0 exclusive tests found, but that would entail modifying the various reporters.

It'd also help if I added debug output so you can see the tests are being run in exclusive mode.

Agreed. Mind opening issues for this? Can be follow-up improvements.

@novemberborn
Copy link
Member Author

Agreed. Mind opening issues for this? Can be follow-up improvements.

Sure. Anything else on this PR?

I'm tracking down the intermittent CI failure but that's not related to this PR.

@sindresorhus
Copy link
Member

Looks good. Land it ;)

A second argument can be passed to Api#run(). If true the tests will be run
in exclusive mode, regardless of whether exclusive tests are detected.

The Api now remits the 'stats' event from the forks.

The watcher keeps track of exclusive tests. If all test files that contained
exclusive tests need to be rerun it runs them without forcing exclusive mode.
This means the exclusivity is determined by the tests themselves.

If a test file, containing exclusive tests, is not one of the files being rerun,
it forces exclusive mode. This ensures only exclusive tests are run in the
changed files, making .only sticky.

If all test files that contained exclusive tests are removed, sticky mode is
disabled. The same happens if there are no more exclusive tests after a run.

Fixes #593.
novemberborn added a commit that referenced this pull request Mar 10, 2016
Make .only sticky in watch mode
@novemberborn novemberborn merged commit 83ab1e4 into master Mar 10, 2016
@novemberborn novemberborn deleted the sticky-only branch March 10, 2016 14:41
@novemberborn
Copy link
Member Author

Maybe it should report 0 exclusive tests found, but that would entail modifying the various reporters.

It'd also help if I added debug output so you can see the tests are being run in exclusive mode.
Agreed. Mind opening issues for this? Can be follow-up improvement

#635 #636

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants