Skip to content

Make runner methods chainable. #243

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

Closed
wants to merge 14 commits into from

Conversation

jamestalmage
Copy link
Contributor

This flattens the entire runner.tests data structure into a single array.

Elements of the array are then chosen using runner.select(selectorObject).

The selector currently just compares its own properties to each test, and selects those that have matching properties. It will be easy to expand with groupMembership queries, etc.

@@ -5,6 +5,7 @@ var Promise = require('bluebird');
var hasFlag = require('has-flag');
var Test = require('./test');
var send = require('./send');
var objectAssign = require('object-assign');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: We probably should look in to replacing some of these polyfills with core-js equivalents. Hopefully cutting down on duplicate downloads for users.

@jamestalmage
Copy link
Contributor Author

I'm getting close to turning this into some cool chaining functionality.

only.serial(...)
serial.only(...)
skip.serial(...)
afterEach.skip(...)
...
etc.

I need sleep now though. More tomorrow.

@sindresorhus
Copy link
Member

I like where this is going :)

Chaining is an important step as it will let you skip a test without losing the type of test. Right now you can't have a skipped test that is serial. You have to remove the serial part, which loses some important information.

@jamestalmage
Copy link
Contributor Author

OK - saw my error - chaining should now work.

This needs some cleanup (see the various TODO's), documentation, and more test coverage.

I should be able to wrap it up this weekend.

When it comes time to make nested groups, we will continue along the same pattern here, but replacing the base _addTest with one that merges the group settings as well (so you can skip an entire group, or set serial as the default for a group for example).

I also need to add the inverse of most of the boolean flags. (i.e. concurrent = {serial: false}). That would be used for making a few tests concurrent in a group of tests where the default is serial.

In all, I think doing it this way is going to open up some really cool stuff.

OK - bedtime for real now.

@jamestalmage
Copy link
Contributor Author

I am starting to think all these flags belong on a metadata object, i.e.:

test.skipped => test.metadata.skipped
test.exclusive => test.metadata.exclusive

Otherwise we need to always be careful the metadata properties never share a name with a useful property. (For example we can not use test.skip as the metadata property, because test.skip() is the method that sets that flag).

@jamestalmage jamestalmage changed the title runner.js: simplify test data structure Make runner methods chainable. Nov 21, 2015
@jamestalmage
Copy link
Contributor Author

Otherwise we need to always be careful the metadata properties never share a name with a useful property

Useful properties are going to be hard to predict as well, especially once we enable custom assertions.

jamestalmage added a commit to jamestalmage/ava that referenced this pull request Nov 22, 2015
runner.js: simplify test data structure

split hooks to their own file

auto-generate runner.addXXX functions

add chaining

drop `map-obj` as a dependency (I did not end up needing it).

refactor tests to use chained methods

simplify chain builder

add chainable tests

make all hooks skippable

test title of skipped test

move test flags to metadata object, instead of right on test

remove unused property
jamestalmage added a commit to jamestalmage/ava that referenced this pull request Nov 23, 2015
runner.js: simplify test data structure

split hooks to their own file

auto-generate runner.addXXX functions

add chaining

drop `map-obj` as a dependency (I did not end up needing it).

refactor tests to use chained methods

simplify chain builder

add chainable tests

make all hooks skippable

test title of skipped test

move test flags to metadata object, instead of right on test

remove unused property
return filter[key] === test.metadata[key];
});
}
return this.tests.filter(filterFn);
Copy link
Member

Choose a reason for hiding this comment

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

Just put the filter function inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wish I could use arrow functions 😞

@sindresorhus
Copy link
Member

@jamestalmage Reviewed. This looks very good. Could you update the readme about the methods being chainable?

@vadimdemedes
Copy link
Contributor

I don't really understand the point of Hook class, since it returns a Test instance in test method. @jamestalmage Could you please explain?

Otherwise, I really like the idea behind this PR.

@jamestalmage
Copy link
Contributor Author

It's for beforeEach / afterEach. Which need to be created multiple times. It's just storing the constructor args for later invoke (multiple times)

@vadimdemedes
Copy link
Contributor

@jamestalmage makes sense!

@sindresorhus
Copy link
Member

👍 Landed. Superb work on this one @jamestalmage :)

tumblr_mqmk6j8kkz1qdlh1io1_250

@jamestalmage jamestalmage deleted the simplify-test-arrays branch April 6, 2016 22:45
@ghost ghost mentioned this pull request Aug 8, 2016
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.

3 participants