Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add support for Jasmine-like "waitsFor" and "runs" #193

Open
rosenfeld opened this Issue Jun 10, 2012 · 19 comments

Comments

Projects
None yet
5 participants

Consider code like this using Jasmine (CoffeeScript for brevity):

$('button#btn').click()
waitsFor -> $('.ui-dialog').length
runs -> $('button:contains(Save)').click()
waitsFor -> fakeServer.requests.length
runs -> fakeServer.respondToNextRequest()

This is how it would have been coded without support for waitsFor and runs in Buster.js:

$('button#btn').click()
waitsForDialog = ->
     (setTimeout waitsForDialog, 10; return) unless $('.ui-dialog').length
     $('button:contains(Save)').click()
     waitsForRequest()
waitsForRequest = ->
    (setTimeout waitsForRequest, 10; return) unless fakeServer.requests.length
    fakeServer.respondToNextRequest()
    done()
waitsForDialog()

I find this much harder to read and maintain and with duplicate logic.

Could you please consider adding something like waitsFor and runs to Buster.js?

Owner

cjohansen commented Jun 11, 2012

CoffeeScript for brevity

That brevity is confusing me ;) Please post code examples in JavaScript.

I don't really understand the role of runs, but I get the idea behind waitFor, which I think is a worthwhile feature. How does that relate to the existing timeout? Timeout like any other test?

No problem, I should stop being so lazy, after all it is just a matter of copying "function(){}" and pasting it all around the examples... :)

In non-blocking runtime you basically have two options for waiting for events:

  1. Use a callback (like the "done" in current Buster.js approach)
  2. Use a queue of functions that should be run in a certain order and check for conditions before running the next item in the queue. Then you poll for the conditions to be met until you can run the next item.

The callback approach will require a bunch of named function declarations if you have lots of conditions to be run in different order like:

  1. run some code
  2. wait for condition to be met
  3. run more code
  4. wait for more conditions
  5. write your assertions.

So with Buster.js approach, this would mean:

timeTick = 10; // ms
"some test": function(done) {
  runSomeCode();
  var timeout = new Date().getTime() + TIMEOUT_INTERVAL;
  var waitsForFirstCondition = function() {
    if (new Date().getTime() > timeout) throw "Timed out";
    if (firstCondition.isNotMet()) {
      setTimeout(waitsForFirstConditon, timeTick);
      return;
    }
    timeout = new Date().getTime() + TIMEOUT_INTERVAL;
    runMoreCode();
  };

  var waitsForSecondCondition = function() {
    if (new Date().getTime() > timeout) throw "Timed out";
    if (secondCondition.isNotMet()) {
      setTimeout(waitsForSecondConditon, timeTick);
      return;
    }
    assertSomething();
    done();
  };

  waitsForFirstCondition();
}

By contrast, take a look at Jasmine documentation for how runs-waitsFor work for such case:

http://pivotal.github.com/jasmine/#section-Asynchronous_Support

I don't really understand why the first "runs" is required in that example, but I had problems in my own tests because I wasn't using "runs" before the first "waitsFor". I guess Jasmine won't block examples in the sense that it would wait for one to finish before starting another one. But I don't really think the first "runs" call should be necessary. I mean, it would work for that specific example, but if you're relying on examples running order you'll find some issues if you don't put your initial code inside a "runs" block as well in Jasmine.

The pseudo-code would be something like this:

  1. run some code
  2. add condition to queue - waitsFor
  3. add more code to queue - runs
  4. add more condition to be tested after item 3 - waitsFor
  5. test for assertions after condition 4 is met

The test runner wouldn't consider the test as finished until the queue is emptied.

Sorry, I'm not sure if I was clear enough... JavaScript code confuses my mind because it is too big to see in a single screen ;)

Also, I forgot to say that Jasmine will require the timeout interval as the last parameter while I'd prefer Buster.js to use a configurable default one if no interval is provided. I'd also like to have some default message like "expected condition has timed out" if no message is given...

Owner

augustl commented Jun 29, 2012

It seems we should probably have a buster-jasmine extension, similar to buster-jstestdriver, for 100% jasmine runner compatibility.

I don't really think it is useful just for Jasmine compatibility. The current way Buster.js handles async tests is not great for lots of cases...

Owner

augustl commented Jun 30, 2012

What if we just add the feature from issue 194, with one little addition?

buster.testCase("My tests", {
    "my test": function () { .... },
    "other test": function () { .... },
    "logging in": buster.testContextSeq([
        "setUp", function () {
            // ...
        },
        "creating user", function (done) {
            // Create the user...
        },
        "authenticating", function (done) {
             // ....
         },
        "setUp", function () {
            // ...
        },
         "can only see its own data", function () {
         }
     ]);
});

Since it's an array, we can have multiple "setUp". We also need a separate thing for that, so we can know that the setUp function should not yield error if there are no assertions there. The nyou can just use the regular setUp async stuff if your setup is, well, async.

What do you think?

Sorry, but I didn't get it. First, you wouldn't need a "setUp" to differentiate a setup from a test. If a function is preceded by a string, then it is a test. Otherwise it would be used for setting up some state.

But anyway, I don't think you understood exactly what waitsFor does.

I don't always know when to call done(). For example, if I run $('button#click-me').click(), I have no idea how jQuery will handle this. The only information I know for sure (and that is what really matters to me) is that a dialog should pop up after I click that button.

So, I have to wait for the dialog to pop up until I can proceed. This usually happens by polling in a certain interval until a timeout is reached.

That is the purpose of waitsFor. I only need to provide it a condition and I don't have to implement the polling logic over and over by myself.

Owner

cjohansen commented Jul 3, 2012

I'm not a big fan of the alternating key/value array. I'd rather opt for something on a higher level, maybe like:

buster.feature("A feature", function (given, when, then, and) {
    given("Something", function () {
        // ...
    });

    when("I do the thing", function () {
        // ...
    });

    and("I dig it", function () {
        // ...
    });

    then("I see it", function () {
        // ...
    });

    and("I hear it", function () {
        // ...
    });
});

This would be awesome! I've used this syntax once and I loved it:

http://www.easyb.org/storyexmpls.html

I'm totally in favor of adding such support, thanks!

Member

magnars commented Jul 3, 2012

It would still need some sort of waitFor with a predicate and a timeout, I guess? I for one would find that very useful.

@magnars could you please demonstrate how waitsFor would help you in a way given-when-and-then-and wouldn't?

I guess each of the functions could use a done argument for that purpose...

About the timeout, I guess the default timeout could be overriden as the second argument for given/when/and/then functions.

Member

magnars commented Jul 3, 2012

Which of given/when/then would implement the polling behavior your requested above? I'm just not seeing it. Maybe an example with the function bodies filled in would help. :-)

Yes, you're right. I tried to write the example and I noticed that we would still need a waitsFor for polling :)

Owner

cjohansen commented Jul 3, 2012

My bad, my suggestion is for a solution for #194, not this one. Sorry.

Owner

cjohansen commented Jul 9, 2012

Looked into this, and I agree that waitsFor looks very useful. I propose we add this:

"some test": function (done) {
    done.wait("Some explanation", pollingFn);
}

I don't see any reason to put in another timeout, so we should just rely on the test timeout. The polling function will be called in an interval until it returns truthy, or the test times out.

@ghost ghost assigned cjohansen Jul 9, 2012

this is limiting in two ways. First, the timeout parameter is interesting sometimes when you know that if your block didn't return true in 20ms it won't return true in 1s and you're able to fail faster.

The other limit is that you wouldn't be able to have multiple runs/waitsFor per test. Please take a look at the description of this issue for an usage example.

Of course, this is just a suggestion :) I'll be using my own runner since I got it to work yesterday, so this is no longer an issue to me, but I'd suggest you to think a bit more about this API.

@dwittner dwittner modified the milestones: 1.x, 0.7 (B5/RC1) Jul 14, 2015

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