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

Please allow describe can do async with done/promise #2085

Closed
Thaina opened this issue Feb 2, 2016 · 22 comments
Closed

Please allow describe can do async with done/promise #2085

Thaina opened this issue Feb 2, 2016 · 22 comments

Comments

@Thaina
Copy link

Thaina commented Feb 2, 2016

In addition to --delay flag. Describe should also able to use done/promise flow to do async before using it

Such as, I want to do dynamic test for all user in database. I need to query user in database with async first, then each of user will emit their own test case in block

describe(function(done){
   query(function(users){
       users.forEach(function(user,i){
             it("Test : " + user.ID,function() {
                  if(!user.ID)
                        throw new Error("user " + i + has no ID);
             });
       });

       done();
   }); 
});
@ORESoftware
Copy link

You should put the database query in a before() hook and use done() with the before; there is no done with describe, because describe is designed to register all functions synchronously.

this is what before() hooks are for!

@Thaina
Copy link
Author

Thaina commented Feb 7, 2016

@ORESoftware You don't understand the use case
Before Cannot make dynamic it. Because as you said, describe is normally synchronous

Register test case asynchronously Is what I want. lt should be possible. And it actually already possible with --delay

I was just want to propose additional way to do it with done in describe. so it is the same code with it(name, done). describe(name, done) should also available

@ORESoftware
Copy link

I see what you mean. You want something like this:

describe('foo',function(){

    var arr = [];

    before(function(done){

        setTimeout(function(){
            arr.push(1);
            arr.push(2);
            arr.push(3);
            done();
        },1000);

    });

    describe('bar',function(){

        arr.forEach(function(item){

            it('[test]' + item, function(){

            });

        });

    });

});

in the above, no tests will be registered, so I see your problem...yeah I don't know. Someone enlighten us.

@Thaina
Copy link
Author

Thaina commented Feb 7, 2016

@ORESoftware I know it cannot work like that from the start. before() is called only with it(). And nothing could be done to delay registering it() in describe() except --delay flag

mocha really doesn't have this funtionality so I need to create this issue to propose new feature. The enlightening you could get is accept that this is missing feature

@ORESoftware
Copy link

Sure, you may have a point, I can't say for sure since I am not a core contributor to mocha

@ORESoftware
Copy link

One work around that I can think of is to have only one it() for the entire dynamic array. You pass in an array of errors to the done() function for the it(). Just make sure not to throw an error on the first error. Not ideal, but will at least get you closer to what you want. Use the before hook for the db call.

@Thaina
Copy link
Author

Thaina commented Feb 8, 2016

@ORESoftware The advantages of it() is grouping, counting, reporting each test case in beautiful structure. Any work around would destroy benefit of using mocha

If I need to do work around like that. Stop using mocha and just echo the test result is easier

@ORESoftware
Copy link

Sure, sounds like you aren't using anything like Continuous Integration/Continuous Deployment, so you can log whatever you want. If this really is an issue, then you are spot on. I agree with you.

@ORESoftware
Copy link

@Thaina you can look into dependency injection and IoC, and you could wrap your Mocha tests in in the DI container. In other words, what RequireJS/AMD does.

@ORESoftware
Copy link

since there is no sync network I/O in node, the only other way besides --delay is to wrap your entire test in the callback from the db call, that should work.

@Thaina
Copy link
Author

Thaina commented Feb 10, 2016

@ORESoftware I already said from the start that "In addition to --delay flag. Describe should also able to use done/promise"

@ORESoftware
Copy link

I agree, but I can guarantee that mocha as is will never implement that feature (for better or for worse). I am working on a Mocha successor that will have the delay feature that you are looking for, if you are interested in checking it out.

@ORESoftware
Copy link

@Thaina check this out

https://github.com/ORESoftware/suman

there is the delay/done functionality you want in the describes

@danielstjules
Copy link
Contributor

Looks like his should be possible with the --delay flag :)

query(function(users) {
  describe('foo', function() {
    users.forEach(function(user,i){
      it('Test : ' + user.ID, function() {
        if(!user.ID) throw new Error("user " + i + has no ID);
      });
    });
  });

  run();
});

@Thaina
Copy link
Author

Thaina commented Feb 16, 2016

@danielstjules I already said from the start that "In addition to --delay flag"

Do you understand this is feature request for better way to do things than --delay flag

@danielstjules
Copy link
Contributor

Sorry, I don't think we'd add this behavior to the BDD interface since the desired outcome is already possible.

@Thaina
Copy link
Author

Thaina commented Feb 16, 2016

@danielstjules It's like you said that we don't need mocha because desired outcome is already possible by console.log

@boneskull
Copy link
Contributor

@ORESoftware Please do not use Mocha's issue tracker to advertise competing projects, thanks

@ORESoftware
Copy link

sure as you can see I tried my best to have mocha fit his needs, he insists
on specific behavior that is simply not in this lib
On Feb 17, 2016 9:17 AM, "Christopher Hiller" notifications@github.com
wrote:

@ORESoftware https://github.com/ORESoftware Please do not use Mocha's
issue tracker to advertise competing projects, thanks


Reply to this email directly or view it on GitHub
#2085 (comment).

@boneskull
Copy link
Contributor

@Thaina @ORESoftware

It's currently not, no. I agree that it'd be nice to have. Unfortunately, it's not a top priority, and we don't have the resources to do this ourselves at the moment, so the change would have to come from a PR. Even then, we're trying to avoid new features due to the associated maintenance burden. It's also a fairly severe change to the test runner, which is why I implemented --delay as a workaround.

All of my resources are going into a prototype for the next phase of Mocha, which should have many of the more popular feature requests. For those that are not included in core, the new version is based around a Hapi-like plugin system, so users can write whatever they need.

@Thaina
Copy link
Author

Thaina commented Feb 18, 2016

@ORESoftware You are the one who insist on distracting this thread from the start instead of understand that This thread is about FEATURE REQUEST

Requested feature surely does not exist yet. If it exist why I need to request? Why you can't understand this very simple logic?

Man, I will never use product from a guy who don't understand what feature request is. Because when I go ask that guy, they will try to do anything but implement the feature that should be there, like you are trying to do

@boneskull
Copy link
Contributor

I'm going to lock this as the conversation has become unproductive.

@mochajs mochajs locked and limited conversation to collaborators Feb 18, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants