mocha icon indicating copy to clipboard operation
mocha copied to clipboard

Please allow describe can do async with done/promise

Open Thaina opened this issue 8 years ago • 20 comments

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();
   }); 
});

ps. This thread is copied from #2085 . It was ruin by a someone and was locked. And I need to make it reopen to ask for something

@boneskull I know that it would not be implemented immediately. That's what software development process is

What I would like you to do is consider you will never do it. Or will be doing it in which version. So I would consider move to another tool or make it by myself

ps2 @boneskull if you just not hurry lock that thread I would not need to waste another thread like this

Thaina avatar Feb 18 '16 04:02 Thaina

@Thaina I locked it because you were becoming belligerent. It's difficult to infer tone in written comments, but I'm sensing some of the same here. Please be polite when posting in issues.

We may be able to implement this in the next major (nothing imminent). I'm not sure it's wise to merge any feature PRs at this point, however. Feel free to implement it yourself in a fork, or use another tool.

boneskull avatar Feb 24 '16 19:02 boneskull

I've been using describe->before(done) to accomplish this, and it seems to be working just fine.

jdmarshall avatar Apr 01 '16 18:04 jdmarshall

It is working, but it's inconvenient. describe('', async () => {} ) will definitely be a step forward!

Related: #2975 #2085

typeofweb avatar Mar 13 '19 12:03 typeofweb

:+1:

pawelngei avatar Mar 13 '19 12:03 pawelngei

Enable this eslint-plugin-mocharule.

We backed out the deprecation warning in #3744 due to problems dealing with CoffeeScript's implicit return, but describe's function is just a namespace -- don't add async to it!

plroebuck avatar Mar 14 '19 11:03 plroebuck

FWIW the use case here is not to run tests in parallel, but rather to dynamically generate tests from data which is retrieved via asynchronous I/O operations. --delay gets us part of the way there, but it is not convenient for certain use cases.

boneskull avatar Mar 20 '19 21:03 boneskull

Is the problem with the lack of async or the lack of ability to register new tests late?

jdmarshall avatar Mar 25 '19 22:03 jdmarshall

Registering tests late works perfectly in the "before" with a dummy "it" test. So that use case is fully supported (admittedly a bit hacky).

simlu avatar Aug 23 '19 15:08 simlu

@simlu is there any case that you could post an example of registering tests late?

betweenbrain avatar Sep 19 '19 16:09 betweenbrain

@betweenbrain Sure thing!

This uses node-tdd because the test setup and tests make external requests. However it should work similarly with vanilla mocha.

const expect = require('chai').expect;
const { describe } = require('node-tdd');

const loadTestConfigsAsync = async () => { /* ... */ };
const runTest = async (config) => { /* ... */ };

describe('Load Configuration Wrapper', { useNock: true, nockFolder: '$FILENAME__cassettes__setup' }, () => {
  beforeEach(async () => {
    const configs = await loadTestConfigsAsync();

    describe('Generate Tests Wrapper', { useNock: true, timeout: 180000 }, () => {
      configs.forEach((config) => {
        // eslint-disable-next-line mocha/no-nested-tests
        it(`Testing ${config.name}`, async () => {
          const result = await runTest(config);
          expect(result).to.equal('success');
        });
      });
    });
  });

  it('Fetching Configuration Dummy Test', () => {});
});

simlu avatar Sep 19 '19 20:09 simlu

Thank you @simlu!

betweenbrain avatar Sep 20 '19 14:09 betweenbrain

My very humble 2c... I just spent 1.5h on this (because I am an idiot). I agree that describe() can be seen as a "namespace" and therefore shouldn't wait for promise resolution. I tried to fix this myself, but Mocha's code is quite complex, and I wouldn't be 100% about the repercussions of such a chance.

What I can say is... maybe let's make it clearer in the documentation, or even (if technically possible) add a big fat warning in case describe() returns a promise, so that others don't get caught in this :D

Again my humble 2c. Sorry for the bother and for the noise.

mercmobily avatar Dec 18 '21 12:12 mercmobily

See #3243

jleedev avatar Dec 18 '21 13:12 jleedev

This is strange... I have a describe() function defined with async, and doing async stuff, and things just don't work. And yet that was merged in 2018... surely it would have made it to release? (Confused)

mercmobily avatar Dec 18 '21 13:12 mercmobily

Regression perhaps? What version are you using?

jdmarshall avatar Dec 23 '21 00:12 jdmarshall

Alright so try this...

const chai = require('chai')
const expect = chai.expect

const asyncFunction = async () => { console.log('an async function') }

describe(`Initialising tiny memory data server`, async function () {

  before(async function () {
    console.log('****************BEFORE******************')
  })

  describe(`All tests starting`, async function () {

    // COMMENT OUT THE NEXT LINE, AND YOU WILL SEE THE before() FUNCTION MAGICALLY WORK
    // await asyncFunction()


    it('One', async function () {
      expect(null).to.be.null
    })


    it('Two', async function () {
      expect(null).to.be.null
    })
  })

})

By trying this code, you will see that if you comment out the line after // COMMENT OUT THE [...] line, the before() function will actually tun. Please note that there are no warnings in any case.

Do you want me to open a new bug report for this? Or am I doing something embarrassingly stupid?

mercmobily avatar Jan 28 '22 10:01 mercmobily

Marking a function as async doesn't make it asynchronous. That's a fundamental problem with Javascript that makes async/await have a smaller feature set than green threads (where you can at least do cooperative multitasking by yielding in the middle of a long operation).

Everything before the first await call always runs sequentially. The context only yields at the first await. So all of your 'it' functions are called before describe() returns, meaning that they are all listed in the suite prior to the runner starting.

To prove this, move your await between the two tests. I believe you will find that one gets defined and the other does not.

jdmarshall avatar Jan 28 '22 18:01 jdmarshall

Declaring a function "Async" just means that it will return a promise. The surrounding code will need to "await" (or not...)

Honest hat on, I am having difficulties understand this part:.

Everything before the first await call always runs sequentially. The context only yields at the first await. So all of your 'it' functions are called before describe() returns, meaning that they are all listed in the suite prior to the runner starting.

When you say "all of your functions are called before describe()", do you mean in my specific example? Or in a working example?

I mean, in my naive head, if whatever calls describe() actually awaits for it, then the it() statements will run by the time the promise resolves and everything should work ..

The use case here is where the suite actually needs to retrieve some data asynchronously. It can be done in a before() function, and it's fine, but the code above is unexpected behaviour.

mercmobily avatar Jan 28 '22 23:01 mercmobily

async function math(x, y) {
    let result = x + y;
    console.log(result);
    return result;
}

async function main() {
    let answer = math(4, 6);
    console.log("....calculating");
    let result = await answer;
    console.log("Answer was", result);
}

If you run this, the output will be:

10
....calculating
Answer was 10

Anything in the async function that happens before the first await, happens before the promise is returned. If you have nothing async in your function, it will run all the way to the end before the caller runs another instruction.

So, an async describe() that contains no async logic will succeed in defining all of the tests. An async describe that defines some tests and then does an async operation, will only register the tests prior to that block of code. One that starts with an async function will register no tests at all.

jdmarshall avatar Feb 01 '22 03:02 jdmarshall

I realise that. But isn't this solved by having the caller of the describe function, within Mocha, to await for the describe function, so that the describe() function will register all of the test before Mocha progresses to run them? I apologise for the noise...

On Tue, 1 Feb 2022 at 11:38, Jason Marshall @.***> wrote:

async function math(x, y) { let result = x + y; console.log(result); return result; }

async function main() { let answer = math(4, 6); console.log("....calculating"); let result = await answer; console.log("Answer was", result); }

If you run this, the output will be: 10 ....calculating Answer was 10

Anything in the async function that happens before the first await, happens before the promise is returned. If you have nothing async in your function, it will run all the way to the end before the caller runs another instruction.

So, an async describe() that contains no async logic will succeed in defining all of the tests. An async describe that defines some tests and then does an async operation, will only register the tests prior to that block of code. One that starts with an async function will register no tests at all.

— Reply to this email directly, view it on GitHub https://github.com/mochajs/mocha/issues/2116#issuecomment-1026448169, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQHWXXSGQVS4MCR6DKIJFTUY5ISFANCNFSM4B3TBRCA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

mercmobily avatar Feb 01 '22 04:02 mercmobily

See also: #1431 & #3106 & #3593 & #4588.

JoshuaKGoldberg avatar Dec 28 '23 00:12 JoshuaKGoldberg