ben-page / node-test

A simple, asynchronous test runner for Node.js.
https://www.npmjs.com/package/node-test
MIT License
4 stars 1 forks source link

What about simplifying and isolating the object? And hook issues? #14

Open ghost opened 8 years ago

ghost commented 8 years ago

In the suite.js you have almost the same identical code, and also idential code. Say your this.test is almost idental to the same function under this.serial except for the options. All hooks have almost similar code too except for the name. What about gathering it into it's own object and only expose that object? In that way no one could get access to the private part of the prototype, only the parts belonging to the exposed object. Similar things as AVA does, except they also add chaining to the exposed object.

Example:

{
beforeEach...
test...
serial...
}

You will not need to change the API if you do that change.

I also noticed when it comes to the hooks there are code that never exist, and code that never are invoked so the delays isn't working. This one. validateFailure has no effect in your code, so that check will always be false.

ben-page commented 8 years ago

I don't really understand what you mean by this:

I also noticed when it comes to the hooks there are code that never exist, and code that never are invoked so the delays isn't working. This one. validateFailure has no effect in your code, so that check will always be false.

But validateFailure has been removed from the project. Maybe you were testing with the latest commit, but not a released version?

ghost commented 8 years ago

It hasn't been removed before now. Look at the Master branch: https://github.com/ben-page/node-test/blob/master/lib/hook.js#L25

And this readme stuff confused me: https://github.com/ben-page/node-test#error-validation

I guess validateError isn't a real function?