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

add ability to delay root suite; closes #362, closes #1124 #1439

Merged
merged 1 commit into from
Feb 16, 2015

Conversation

boneskull
Copy link
Contributor

This allows delayed root suite execution with flag --delay.

While I was in there I refactored some to reduce LOC (see lib/interfaces/common.js).

It works like this:

  • if you specify --delay, the Runner will wait until the root suite has emitted a run event
  • in addition, --delay will put a function run() in the global context
  • once you have done your async business and generated your suite(s), call run().
  • this emits the event on the root suite, and it is executed.

See the unit test for an example.

Also rebuilt /mocha.js.

@dasilvacontin
Copy link
Contributor

(the coffee is strong with this one)

@boneskull boneskull added the status: waiting for author waiting on response from OP - more information needed label Nov 21, 2014
This was referenced Nov 21, 2014
@dasilvacontin
Copy link
Contributor

LGTM 👍

gh-pages and docs should be updated with the new flag.

@thecyman
Copy link

👍

@boneskull
Copy link
Contributor Author

todo: update docs

@boneskull
Copy link
Contributor Author

todo: rebase

@boneskull
Copy link
Contributor Author

I think I have a couple tweaks to the test too I need to push.

@boneskull boneskull added TO-MERGE and removed status: waiting for author waiting on response from OP - more information needed labels Dec 15, 2014
@ainthek
Copy link
Contributor

ainthek commented Feb 9, 2015

is this finished ? Please.
I also do not understand why simply not making describe(function(done)) signature to signal, that
builder of suite is async and needs to wait();

@boneskull
Copy link
Contributor Author

@ainthek It's not finished yet. I'm not sure what you're suggesting here.

@ainthek
Copy link
Contributor

ainthek commented Feb 10, 2015

boneskull: I do not get the idea with delay() and run() and wanted to test it.
When looking at the proposal of API I would suggest

describe(function(done){
asyncData(unroll);
function unroll(err,data){
data.formEach(function(){
it();
});
done();
}
});

simple and symmetric with it, before and others supporting done and async nature

@ainthek
Copy link
Contributor

ainthek commented Feb 10, 2015

Another trick to solve 'async unroll' until some reasonable support is available:

var assert=require("assert");
describe("Async unroll is possible", function(){

    function unrollDataAsyncTask(done){
        // fs, request whatever async here, callback or promise
        setTimeout(function(){
            done(null,[1,2,3,4]);
        }, 1000);
    }

    var addTest=this.addTest.bind(this);
    before(function(done){
        this.timeout(2000);
        // call asynct task and generate tests
        unrollDataAsyncTask(function(err,datas){
            datas.map(_it).forEach(addTest);
            done(); 
        });
    });
    // make at least on emethod, so before gets called
    it("Unroll Data shall exist", function(done){
        unrollDataAsyncTask(function(err,datas){
            assert(datas.length>0);
            done();
        });
    });

    // then this will become many it() registered in before();
    function _it(data){
        return it("test for "+data, function(){
            // ...
        });
    }

@boneskull
Copy link
Contributor Author

@ainthek I think you misunderstand. What you wrote is something Mocha already supports. I'm also not sure what you mean by "unroll".

What Mocha does not support is delaying execution of the initial describe block. If you attempt this without the change, currently your tests will not run.

See this unit test for an example (the unit test needs some tweaks, but that's the general idea).

@boneskull boneskull force-pushed the issue/362 branch 2 times, most recently from 51ccce9 to b66c5ef Compare February 10, 2015 06:20
@ainthek
Copy link
Contributor

ainthek commented Feb 10, 2015

why would you need to delay execution of decribe ? one reason is building the list of tests dynamically (based on some asyncly retrieved data, which is called unroll in other fmwks) or waiting for something else before executing function in describe, both can be currently achieved by before. yes i posted what can be already achieved. what and why you are trying to achieve here ? waiting before describe itself is called not before function in describe is called ? why ? really not clear nor from discussion nor from test case. will have look again.

Sent from my iPad

On 10.2.2015, at 6:24, Christopher Hiller <[email protected]mailto:[email protected]> wrote:

@ainthekhttps://github.com/ainthek I think you misunderstand. What you wrote is something Mocha already supports. I'm also not sure what you mean by "unroll".

What Mocha does not support is delaying execution of the initial describe block. If you attempt this without the change, currently your tests will not run.

See this unit testhttps://github.com/boneskull/mocha/blob/issue/362/test/delay/delay.js for an example (the unit test needs some tweaks, but that's the general idea).


Reply to this email directly or view it on GitHubhttps://github.com//pull/1439#issuecomment-73645392.

@dasilvacontin
Copy link
Contributor

@ainthek, please check the relevant threads mentioned in the title: #362 and #1124.



modifies http tests to avoid race conditions
boneskull pushed a commit that referenced this pull request Feb 16, 2015
@boneskull boneskull merged commit ea9951c into mochajs:master Feb 16, 2015
@foxx
Copy link

foxx commented Apr 2, 2015

I'm so happy to have found this feature, it's a shame it took 2 years to get included but it's here now and that's all that matters. Thank you @boneskull !

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.

5 participants