Async tests where the only assertion is "done is called" #151

Closed
augustl opened this Issue Apr 9, 2012 · 5 comments

Comments

Projects
None yet
5 participants
@augustl
Member

augustl commented Apr 9, 2012

Some of my tests look like this:

"test foo": function (done) {
    assert(true);
    myThing.on("foo", done);
    doSomething();
}

I need the assert(true) to avoid a zero assertions test failure, since I don't have any assertions in my test. The fact that done is called is the only actual test.

Possible solution:

"test foo": function (done) {
    myThing.on("foo", assert(done));
    doSomething();
}
@sveisvei

This comment has been minimized.

Show comment
Hide comment
@sveisvei

sveisvei Apr 11, 2012

+1

+1

@cjohansen

This comment has been minimized.

Show comment
Hide comment
@cjohansen

cjohansen Apr 13, 2012

Member

My gut feeling says "bad test". Do you have some actual examples of where you're doing this? Maybe each test is doing too little?

Member

cjohansen commented Apr 13, 2012

My gut feeling says "bad test". Do you have some actual examples of where you're doing this? Maybe each test is doing too little?

@ThisIsMissEm

This comment has been minimized.

Show comment
Hide comment
@ThisIsMissEm

ThisIsMissEm Oct 8, 2012

I actually need similar, where I have bunch of stuff in the test, but I want to test that an event is actually emitted. Currently I'm doing:

        var emitted = false;
        var handle = this.instance.on("test-event", function(){
          emitted = true;
          handle.remove();
        });

        this.instance.foo.emit("something-that-in-turn-triggers-test-event");
        assert(emitted);

Would be nicer to do something like:

        assert.emits(this.instance, "test-event" [, count? ]);
        this.instance.foo.emit("something-that-in-turn-triggers-test-event");

        done();

I actually need similar, where I have bunch of stuff in the test, but I want to test that an event is actually emitted. Currently I'm doing:

        var emitted = false;
        var handle = this.instance.on("test-event", function(){
          emitted = true;
          handle.remove();
        });

        this.instance.foo.emit("something-that-in-turn-triggers-test-event");
        assert(emitted);

Would be nicer to do something like:

        assert.emits(this.instance, "test-event" [, count? ]);
        this.instance.foo.emit("something-that-in-turn-triggers-test-event");

        done();
@dwittner

This comment has been minimized.

Show comment
Hide comment
@dwittner

dwittner Aug 6, 2013

Member

I think the solution of augustl is sufficient in most cases. If you really need to verify more than the callback is called at all, you can write your own assertion:

buster.assertions.add("emits", {

    assert: function(func, event, callback) {

        myThing.on(event, callback);
        func.call();

        return true;
    }
});

testCase("async", {

    "callback is called correctly": function(done) {

        var callback = this.spy(function() {
            if (callback.callCount === 2) {
                done(function() {
                    callback.alwaysCalledWith("bar");
                })();
            }
        });

        assert.emits(doSomething, "foo", callback);
    }

});

Unfortunately the code:

                done(function() {
                    callback.alwaysCalledWith("bar");
                })();

doesn't work here (don't ask me why), so i had to change it to:

            if (callback.callCount === 2 && callback.alwaysCalledWith("bar")) {
                done();
            }

This only leads to a timeout notification and we don't get a meaningful error message. Maybe someone knows how to solve that problem.

@augustl, @cjohansen: Generally i don't think Buster.JS needs to provide a predefined assertion for that kind of tests. If you agree, i suggest to close the issue.

Member

dwittner commented Aug 6, 2013

I think the solution of augustl is sufficient in most cases. If you really need to verify more than the callback is called at all, you can write your own assertion:

buster.assertions.add("emits", {

    assert: function(func, event, callback) {

        myThing.on(event, callback);
        func.call();

        return true;
    }
});

testCase("async", {

    "callback is called correctly": function(done) {

        var callback = this.spy(function() {
            if (callback.callCount === 2) {
                done(function() {
                    callback.alwaysCalledWith("bar");
                })();
            }
        });

        assert.emits(doSomething, "foo", callback);
    }

});

Unfortunately the code:

                done(function() {
                    callback.alwaysCalledWith("bar");
                })();

doesn't work here (don't ask me why), so i had to change it to:

            if (callback.callCount === 2 && callback.alwaysCalledWith("bar")) {
                done();
            }

This only leads to a timeout notification and we don't get a meaningful error message. Maybe someone knows how to solve that problem.

@augustl, @cjohansen: Generally i don't think Buster.JS needs to provide a predefined assertion for that kind of tests. If you agree, i suggest to close the issue.

@cjohansen

This comment has been minimized.

Show comment
Hide comment
@cjohansen

cjohansen Aug 6, 2013

Member

I agree

Member

cjohansen commented Aug 6, 2013

I agree

@cjohansen cjohansen closed this Aug 6, 2013

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