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

API Suggestion: Use an explicit callback argument for completion of async specs #178

Closed
AlexJWayne opened this issue Jan 16, 2012 · 14 comments

Comments

@AlexJWayne
Copy link

Reading through the mocha.js docs: http://visionmedia.github.com/mocha/

One awesome feature stands out. Spec blocks can accept an argument, which is a function that can be called to signify the end of an async spec.

describe('User', function(){
  describe('#save()', function(){
    it('should save without error', function(done){
      var user = new User('Luna');
      user.save(function(err){
        if (err) throw err;
        done();
      });
    })
  })
})

To do the same in Jasmine, it might look like this:

describe('User', function(){
  describe('#save()', function(){
    it('should save without error', function(){
      var user, error;

      runs(function() {
        user = new User('Luna');
        user.save(function(err){
          error = err;
        });
      });

      waits(100);

      runs(function {
        expect(error).toBeFalsy();
      });

    })
  })
})

I suppose you could setup a spy, and waitsFor() that spy to be called form the callback as well. But the point is this: you have to set all that up. Using the provided callback instead, cleans up the test greatly.

I assume this works by checking the specFunction.length and if that is one, then we assume it's an async spec and refuse to move on until that callback is called (or spec times out of course). And if it's zero the spec runs synchronously, or can use waits/runs queues as normal.

I've had to train a few people on Jasmine and the waits() and runs() dance is always a sticking point of conceptual understanding. But an explicit, "done with everything in this spec" callback is a far easier API to grasp, and requires far less setup in the spec itself. In my opinion it's easier to learn, write, read and maintain.

@infews
Copy link
Contributor

infews commented Feb 11, 2012

Good idea. I've marked it as "v2" as that's where we're putting syntax changes/updates for now. Thx!

@timbertson
Copy link

+1, would love to see this (I was just about to request the same thing).

@masklinn
Copy link

This also makes promise-based code much simpler to write: the developer can just register the specified deferred into the promise e.g.

it('should save without error', function(done){
    var user = new User('Luna');
    user.save().then(done, done);
});

(nb: that does not work in Mocha if save returns data on success, as its done only expects failure)

the code would be even nicer if it provided a "failure" callback as well, which would use whatever arguments were passed to it as the test failure report:

it('should save without error', function(done, failed) {
    var user = new User('Luna');
    user.save().then(done, failed);
});

@AlexJWayne
Copy link
Author

@masklinn In my opinion a spec should fail in only 2 cases, when an expectation is not met, or an uncaught exception is raised. This third and far less common failure method seems inconsistent.

@jchris
Copy link

jchris commented Mar 6, 2012

I'd like this feature as well. :)

@masklinn
Copy link

masklinn commented Mar 6, 2012

This third and far less common failure method seems inconsistent.

It's not a third failure method. There's an expectation that the promise is resolved (succeeds), if the promise is rejected (fails) then it's a failure.

@waltercacau
Copy link

+1, would love to see this also

@gimmi
Copy link

gimmi commented May 5, 2013

mocha support this, jasmine-node already support it too, Derick Bailey published an extension to support it, so please add this feature to the core

@hakubo
Copy link

hakubo commented May 6, 2013

+1 to that
I use https://github.com/derickbailey/jasmine.async in my projects
Bu this would be so much nicer to have in core

@infews
Copy link
Contributor

infews commented May 6, 2013

This is implemented on master as part of 2.0. This version is not quite ready for prime time, but you can see the direction we are heading.

@infews infews closed this as completed May 6, 2013
@gimmi
Copy link

gimmi commented May 7, 2013

Seems that work on 2.0 branch is not very active, any ETA?

@infews
Copy link
Contributor

infews commented May 8, 2013

There is work going on in a couple of branches. But yes, it moves in fits-and-starts.

@Bartvds
Copy link

Bartvds commented May 14, 2013

I'm not sure of the status here; but a FYI on a nice alternative:

The taskrunner grunt assumes synchronous execution of a task function unless you call var done = this.async(); from inside the function body so it enables async mode for that execution and returns a closure you need to call to continue.

This could be more flexible then passing in the done as argument like in the above proposal, because you can pass arguments to the this.async(), like a message and a timeout delay, You could also call it multiple times and have the runner wait for all the calls to be cleared.

it('haswebservice', function(){
    var done = this.async(5000, 'calling webservice');
    request.send(function(err, res){
        expect(res.ok).toBe(true);
        done();
    });
});

@gimmi
Copy link

gimmi commented May 15, 2013

Like it! This can also be supported in addition to the callback version

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

No branches or pull requests

9 participants