Describe block with async function behaving weirdly

[shashi-raja]

Mocha 3.5
Node 8.4

describe('Test Suite 1', async function(){
  let email = await faker.internet.email();
  it('should print email', function(){
     email.should.not.be.a('string')
   )}
})

Running this test should give

Test Suite 1
   ✓should print email

But its giving
0 passing

Also modifying the above code to

describe('Test Suite 1', async function(){
  let email;
  it('should print email', async function(){
     email = await faker.internet.email()
     email.should.not.be.a('string')
   )}
})

Runs the test but doesn't print the name of describe i.e Test Suite 1
✓should print email
Removing the async from describe and putting it in it works fine

 describe('Test Suite 1', function(){
  it('should print email',async  function(){
     let email = await faker.internet.email();
     email.should.not.be.a('string')
   )}
})

Test Suite 1
   ✓should print email

Also .only and .skip do not work when async function is included in the describe block

[ScottFreeCode]

Async functions are syntactic sugar for returned promise chains, and Mocha's describe blocks do not support (as in waiting for resolution of) returned promises. Failing to warn about it is perhaps less helpful than it could be, but the behavior is pretty much expected under the current design.

For cases where the names of the tests or how many tests there are can only be determined asynchronously, there is the --delay option, although that still requires the entire describe to occur after the asynchronous info is obtained (it still must be called synchronously inside describe before the describe returns).

If you don't need the data to define the tests but only to run them, you can use async before/beforeEach hooks to get it (that would be the obvious way to do these trivial examples), or even async tests with the initial methods to obtain the data written into the start of the test.

If you can come up with a real use case where --delay's limitations are needlessly awkward but it can't just be done with before/beforeEach since the definition of tests depends on it, then a case could be made for adding promise support (from which async function support would automatically be derived) to describe.

[caphale]

@ScottFreeCode , I have similar problem noted above.

Describe('some statement', function(){

before(function(done){

some async function;
done();
});
async.eachSeries(test,function(test,call){
it ('should test---', function(){

});

})

})

It block does not wait till before is complete. ' it' races around .

Now remove the async function , (dynamically forming tests). Before works as expected.

[ScottFreeCode]

Hi @caphale,

I get errors if I copy-paste this code into a new test file and try to run it, so I can't exactly debug what's happening. I don't even know whether by "Now remove the async function..." you mean the asynchronous code in the before or the async.eachSeries -- although it's more important that both might be incorrect, but I cannot necessarily tell how so given my lack of knowledge of where async.eachSeries comes from and what code structure has been replaced with "some async function;" in the before.

I can take some guesses, but these are just guesses:

• Does the before block really look something like this?

  before(function(done) {
    thisFunctionHappensToBeAsync()
    // or
    ;(async function() {
      // setup steps here
    })()
    done()
  })

  If so, either move done inside the async function, or change it to this:

  before(thisFunctionHappensToBeAsync)
  // or
  before(async function() {
    // setup steps here
  })

  The slightly longer "why" is that done tells Mocha that you've finished executing your asynchronous steps and before has completed running; if you call done after you've made the function calls that schedule the asynchronous setup but before those asynchronous steps actually complete, Mocha cannot know that it has to wait for them to complete (because you're telling Mocha that they already have); instead you'd need to call done in (at the end of) the asynchronous steps.
• I'm not familiar with async.eachSeries, but I assume from the name that the callback is not called synchronously, so those its are presumably being created outside the describe suite (since as previously mentioned it must be called before describe returns and not in their own asynchronous callbacks, whether describe is called in an asynchronous callback or not). The following points need to be kept in mind when designing the correct test structure:
  • If you don't need the asynchronous data to determine the number of tests or their names, then call describe and it synchronously instead and get the asynchronous data in an asynchronous or promise-returning before (or even directly in an asynchronous or promise-returning test).
  • If you do need to use asynchronously retrieved data to determine the number or names of its, you must use whatever asynchronous API you have to gather all the data and in a single, final callback call describe and all the its inside it followed by run(), then add the --delay option to running Mocha.

---

P.S. If you wrap your code in:
```js
(code here)
```
...GitHub will format it much better.

[bennypowers]

I want to fetch test data and use it to partially apply a validation function. doing it in beforeEach would defeat the purpose, since I wouldn't have the data outside the scope of the test

suite('muh data', async () => {
  const DATA = await fetch('data.json').then(jsonify);

  // checks that only path has changed
  // original -> path -> obj -> assertion
  const validatePropChanges = curry((original, path, obj) {/* yadda yadda */});

  const validate = validatePropChanges(DATA);

  test('incrementFoo only increments foo', () => {
    const cloned = clone(DATA);
    incrementFoo(cloned);
    cloned.foo.should.equal(DATA.foo + 1);
    validate('foo', DATA)
  });

[Bamieh]

I'd use this approach

const validatePropChanges = curry((original, path, obj) {/* yadda yadda */});

suite('muh data', function() {
   let validate;
   let DATA;
   before(async function() {
     const DATA = await fetch('data.json').then(jsonify);
     validate = validatePropChanges(DATA);
   })


  test('incrementFoo only increments foo', () => {
    const cloned = clone(DATA);
    incrementFoo(cloned);
    cloned.foo.should.equal(DATA.foo + 1);
    validate('foo', DATA)
  });

[stale]

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

[outsideris]

It's not stale because of #3243

[ofrobots]

In TypeScript projects, the lack of ability to use async describe functions leads to the following:

I want to write:

describe('express middleware', async () => {
  const {logger, mw} = await elb.middleware({level: 'info'});
  // ...
}

What I need to write:

describe('express middleware', () => {
  type ThenArg<T> = T extends Promise<infer U> ? U : T;
  type ElbThenType = ThenArg<ReturnType<typeof elb.middleware>>;
  let logger: ElbThenType['logger'];
  let mw: ElbThenType['mw'];

  before(async () => {
    ({logger, mw} = await elb.middleware({level: 'info'}));
  });

It would be quite handy if describe were to wait for the promise to resolve if one was returned. Is that possibility, or is there something fundamental that prevents this from ever happening.

[pronebird]

Is there any plan to fix this? I need async describe to spin off mongo before creating the app and running tests.

[plroebuck]

Why not spin up MongoDB in root-level before(), or use --delay with this.run()?

[pronebird]

@plroebuck --delay could work (ps I realized I haven't seen this option 😮). That's the scenario I have:

describe("Suite", async () => {
  const mongoURI = await spinOffMongoAndReturnConnectionString();
  const configuration = {
    ...readConfigFromEnv(),
    mongoURI
  };
  const expressApp = createTestApp(configuration);

  it('should do magic', async () => {
    const res = await chai.request(expressApp).put('/some/url').send({ ... });
    res.body.should.be.equal(true);
  });
})

ps: I would gladly move mongo setup to global hook that runs once before everything happens.

[plroebuck]

Maybe...

let dbconn;

before(async () => {
  dbconn = await spinOffMongoAndReturnConnectionString();
});

describe("Suite", () => {
  let expressApp;

  before(() => {
    const configuration = {
      ...readConfigFromEnvSync(),
      dbconn
    };
    expressApp = createTestAppSync(configuration);
  });

  it('should do magic', async () => {
    const res = await chai.request(expressApp).put('/some/url').send({ ... });
    res.body.should.be.equal(true);
  });

  after(() => {
    expressApp = null;
  });
})

after(() => {
  dbconn = null;
});

[pronebird]

@plroebuck Thanks. Surely that works except that Flow doesn't know how mocha works so it will likely force me to check if expressApp is null in each occurrence. Doh.. :/

ps: on the side note, chai.request() is not typed properly either so Flow swallows this error so f* it let's move on.

[plroebuck]

LOL...

  it('should do magic', async () => {
    // $FlowMocha
    const res = await chai.request(expressApp).put('/some/url').send({ ... });
    res.body.should.be.equal(true);
  });

".flowconfig"

suppress_comment= \\(.\\|\n\\)*\\$FlowMocha

[alex-at-cascade]

I struggled with this same problem for a while (certain tests mysteriously not running) until I found this issue posted. I had just assumed describe would wait for promises the same way it does. I can work around it, but it would be wonderful if describe could just recognize async functions and behave accordingly.

[plroebuck]

@alex-at-cascade, well at least you'll get a warning from the upcoming release...

[WORMSS]

Just to add to the discussion that our company got caught out by the non-async nature of describe when a developer changed from require to import to match the rest of the system. Sadly this caused no errors, and was only during the addition of .only() by myself that the test reported 0 Passed when I knew there should be 38 that either passed or failed.`

describe.only('aqs.utils', async () => {
  await import('./equals');
  await import('./gas');
  await import('./notEquals');
  await import('./startsWith');
  await import('./wordSearch');
});

We went back to the original and now works as expected, but I think we would prefer to use import

describe.only('aqs.utils', () => {
  require('./equals');
  require('./gas');
  require('./notEquals');
  require('./startsWith');
  require('./wordSearch');
});

[danielbodart]

Is there any plan to fix this in the long term?

[jcollum]

Yeah this behavior is a pain, resulted in a lot of weird race conditions for me.

What worked for me: if you need async, do it in your before blocks.

[craigphicks]

After reading this post I posted the following feature suggestion/analysis -

"Feature async-describe -- Analysis of required design & usage changes" #4641

[cinderblock]

the before block does not seem to be working for me.

describe('my lib', () => {
  let whatever: string;
  before(async () => {
    whatever = await referenceImplementation();
  });
  descripbe(`implementation`, () => {
    it(`should match: [${whatever}]`, () => {
      myImplementation().should.equal(whatever);
    });
  });
});

Regardless, why is there such resistance to async/await?

Technically one can do anything without async/await... but we do it anyway because it's so much nicer. This causes major headaches otherwise.

[WORMSS]

I believe they want to keep the "build up the structure of where the tests are" as synchronous as possible.

[juergba]

Mocha runs in different cycles, we have a description of those in our docs.

In the "parsing" cycle all describe callbacks are excecuted and the test hierarchy (runner) is created. Those callbacks are indeed synchronous. None of the tests/hooks will be run at this time. That's why [${whatever}] is undefined.

Then in the next cycle the runner is launched and whatever should have a value, since async/await does work in hooks.

If you need to run an aynchronous job before runner is created, you can eg. use top-level await as described in our docs (dynamically generating tests).

[cinderblock]

I hadn't see top-level await was available on mocha. That should work for my purposes however TS -> ESM support is still new. Still don't see how it makes sense to prevent this.

[ahilton-godaddy]

@juergba just to add a use case for the reason you may with a async describe block.

I am generating some dynamic tests, and without being able to dynamically describe it blocks, it makes dynamic test generation a difficult thing to do. esm modules still have some rough edges for our implementation

[jcollum]

I thought `it` blocks were async but `describe` blocks were not... Or is that Jest?

…

On Tue, Jan 18, 2022 at 3:22 AM ahilton-godaddy ***@***.***> wrote: @juergba <https://github.com/juergba> just to add a use case for the reason you may with a async describe block. I am generating some dynamic tests, and without being able to dynamically describe it blocks, it makes dynamic test generation a difficult thing to do, and something people do attempt :D — Reply to this email directly, view it on GitHub <#2975 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFMAAWGATYKOL5U263IHQLUWVEOZANCNFSM4DYWOEOA> . 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: ***@***.***>

[cinderblock]

@jcollum You are correct. I'm wondering why describe blocks cannot be made async.

[swordensen]

Just to add to the discussion that our company got caught out by the non-async nature of describe when a developer changed from require to import to match the rest of the system. Sadly this caused no errors, and was only during the addition of .only() by myself that the test reported 0 Passed when I knew there should be 38 that either passed or failed.`

describe.only('aqs.utils', async () => {
  await import('./equals');
  await import('./gas');
  await import('./notEquals');
  await import('./startsWith');
  await import('./wordSearch');
});

We went back to the original and now works as expected, but I think we would prefer to use import

describe.only('aqs.utils', () => {
  require('./equals');
  require('./gas');
  require('./notEquals');
  require('./startsWith');
  require('./wordSearch');
});

wanted to add to this by mentioning that ESM is now the official standard for Node.js and require is no longer available within ESM scope.

I do love splitting up my tests this way and I hope Mocha can support asynchronously importing test files in the future. IMHO this issue should be opened back up.

[edit]: it also looks like people were looking at Jest to support the same thing for similar reasons => jestjs/jest#2235

[cinderblock]

That mocha reports "all good" after "0 Passed" (because it thought no tests had been defined) is one of the problems here. It's easy to blindly switch to imports and see "all green" on tests, not realizing that tests were not even being run.

@Mikkal24 Have you see the (new in v9.1) --fail-zero option?

[WORMSS]

That --fail-zero would only help if ALL your tests were in that one describe..
Only about 400 test were in the aqs.utils location.
The other 4200 tests were scattered throughout many other files.
As I said back in 2018, it was ONLY because I used .only( that I even noticed this problem at all.

[swordensen]

That mocha reports "all good" after "0 Passed" (because it thought no tests had been defined) is one of the problems here. It's easy to blindly switch to imports and see "all green" on tests, not realizing that tests were not even being run.

@Mikkal24 Have you see the (new in v9.1) --fail-zero option?

I am aware that that switching to imports results in the tests not executing. I assume this is because the import function returns a promise and describe does not support promises which was the purpose of this issue.

I just wanted to tack on to what @WORMSS said in 2018 because while import was a "nice to have" it's now required in the official standard of Node.js since require doesn't exist in ESM.

I can only hope Mocha devs will support this syntax in the future.

describe("some group", async ()=>{
    await import("myModule.js");
    await import("myOtherModule.js");
})

this would help a lot with maintaining large code bases. (TBF I've only got like 100 test files, I would be desperate for this if I had 4200 lol)

[farwayer]

@juergba it looks like more and more people are falling into this trap. It's a bit dangerous when the testing tool just stops running part of the tests without any errors or warnings. And the lack of support for asynchronous describe violates the golden rule of predictable behavior. If async it is supported, it's reasonable to assume that async describe should also works.

For me, the main reason for using async describe is to import ESM modules. And even if there are several stages of test execution, why not wait for async describe?

[ViSaturn]

Asynchronous describe would be really useful to me in my situation, I have a recursive function that loops through an object and dynamically imports a file (dynamic imports are async) corresponding to the index, creates an it if the import works, or creates a pending it if the import fails. In mocha I could not get this behavior to work and thought it would look awkward if I had half of the code for my describe.. outside of the describe, I originally just expected that if I returned a Promise, it would allow me to define it's until the promise is resolved, but this is not the case. A before() wouldn't work because the definition of the tests depends on the imports which would be placed in the before() itself, which would not run before I define a test.

Edit: The following solution worked okay for me, still not great tho:

export default async function asyncDescribe(desc: string, run: Function) {
  const its: Record<string, Function | undefined> = {}

  return run((testName: string, func?: Function) => {
    its[testName] = func
  }).then(() => {
    describe(desc, () => {
      for (const [testName, runFunction] of Object.entries(its)) {
        it(testName, runFunction)
      }
    })
  })
}

and you use top-level await on the function

[krzkaczor]

@boneskull can we reopen this as it's still not fixed?

[mcdurdin]

juergba commented on Jan 3, 2022:

Mocha runs in different cycles, we have a description of those in our docs.

In the "parsing" cycle all describe callbacks are excecuted and the test hierarchy (runner) is created. Those callbacks are indeed synchronous. None of the tests/hooks will be run at this time. That's why [${whatever}] is undefined.

Then in the next cycle the runner is launched and whatever should have a value, since async/await does work in hooks.

This is helpful information which I could not find in the documentation. There's no reference to 'parsing' anywhere and the asynchronous discussion does not mention the fact that describe() will not accept an async function. So I've opened a PR at #5046, hope that is useful.