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

🚀 Feature: Allow async function in describe (block) #2116

Open
Thaina opened this issue Feb 18, 2016 · 21 comments
Open

🚀 Feature: Allow async function in describe (block) #2116

Thaina opened this issue Feb 18, 2016 · 21 comments
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal

Comments

@Thaina
Copy link

Thaina commented Feb 18, 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();
   }); 
});

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

@boneskull boneskull added the type: feature enhancement proposal label Feb 24, 2016
@boneskull
Copy link
Member

@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.

@jdmarshall
Copy link

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

@typeofweb
Copy link

typeofweb commented Mar 13, 2019

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

Related: #2975 #2085

@pawelngei
Copy link

👍

@plroebuck
Copy link
Contributor

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!

@boneskull
Copy link
Member

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.

@jdmarshall
Copy link

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

@simlu
Copy link

simlu commented Aug 23, 2019

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

@betweenbrain
Copy link

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

@simlu
Copy link

simlu commented Sep 19, 2019

@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', () => {});
});

@betweenbrain
Copy link

Thank you @simlu!

@mercmobily
Copy link

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.

@jleedev
Copy link

jleedev commented Dec 18, 2021

See #3243

@mercmobily
Copy link

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)

@jdmarshall
Copy link

Regression perhaps? What version are you using?

@mercmobily
Copy link

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?

@jdmarshall
Copy link

jdmarshall commented Jan 28, 2022

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.

@mercmobily
Copy link

mercmobily commented Jan 28, 2022

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.

@jdmarshall
Copy link

jdmarshall commented Feb 1, 2022

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.

@mercmobily
Copy link

mercmobily commented Feb 1, 2022 via email

@JoshuaKGoldberg JoshuaKGoldberg changed the title Please allow describe can do async with done/promise 🚀 Feature: Allow async function in describe Dec 27, 2023
@JoshuaKGoldberg
Copy link
Member

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

@JoshuaKGoldberg JoshuaKGoldberg changed the title 🚀 Feature: Allow async function in describe 🚀 Feature: Allow async function in describe (block) Dec 28, 2023
@JoshuaKGoldberg JoshuaKGoldberg added the status: accepting prs Mocha can use your help with this one! label Jan 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests