chaijs / chai

BDD / TDD assertion framework for node.js and the browser that can be paired with any testing framework.
https://chaijs.github.io
MIT License
8.15k stars 698 forks source link

[v5] `assert.isFunction()` fails with async function #1564

Closed ReDemoNBR closed 10 months ago

ReDemoNBR commented 11 months ago

The function assert.isFunction is failing on AsyncFunction on v5.0.0, while on v4.3.10 it was working

A simple scenario is the one below:

describe("Scenario", ()=>{
    it("Should be a function", ()=>{
        async function foo() {
            console.log("Hello World");
        }
        assert.isFunction(foo); // fails on v5.0.0
    });
});

For now I changed my tests to use the JS builtin typeof, like:

assert.strictEqual(typeof foo, "function");`
koddsson commented 11 months ago

Wow! Weird that we haven't hit this before. I don't even see a test for this so it makes sense we didn't catch this in the upgrade. I've created a failing test on a local branch and I'll see if I can make a fix today. I'll keep you posted.

43081j commented 11 months ago

@koddsson its most likely because an async function's tag is AsyncFunction, not Function

presumably the library we used to use just treated them as one

edit:

here you go: https://github.com/chaijs/type-detect/blob/4415ced2c49007f097627515806553e7f649c293/index.ts#L69-L72

koddsson commented 11 months ago

@koddsson its most likely because an async function's tag is AsyncFunction, not Function

presumably the library we used to use just treated them as one

Yup! We refactored type-detect as part of v5. Now the question is if this functionality is actually correct and we should introduce isAsyncFunction to be more granular in assertions.

I put together a draft PR here and wrote down some thoughts: https://github.com/chaijs/chai/pull/1566

asamuzaK commented 11 months ago

There is a similar issue with assert.typeOf(async ()=> {}, 'function').