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

When a fail occurs before a done() call, the error message is wrong #586

Closed
DannyBrown-xx opened this issue Jan 5, 2016 · 12 comments
Closed

Comments

@DannyBrown-xx
Copy link

Given the following code:

 Promise.all(typePromises).then(results => {
     console.log(results)
     expect(results).not.to.contain(false);
     done();
 });

I would expect chai to give me an error saying it expected the array not to contain false, if it does contain false.

However, when this is the case I instead get the following error after 2 seconds:
Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

Clearly my code contains a done() call, and it is called correctly when my test passes. However, when the .not.to.contain detects a failure it is not called.

@lucasfcosta
Copy link
Member

Perhaps your promise isn't being resolved at all when the test fails and that's why the timeout is exceeded.
Try to increase your timeout using this.timeout(60000) just to see if you can get your result after a longer time.

Is your console.log(results) being called when the test fails?

@keithamus
Copy link
Member

Thanks for the issue @dannybrown. Thanks for helping out there @lucasfcosta

The reason this is not doing what you want is because Chai throws errors when an assertion fails. So your code never reaches done(), and the Promise gets turned into an error state - but because you're doing nothing else with the Promise, it never goes anywhere, and subsequently your test is timing out.

You probably want to do the following instead:

 Promise.all(typePromises).then(results => {
     console.log(results)
     expect(results).not.to.contain(false);
 }).then(() => done(), error => done(error));

Which can be shortened to

 Promise.all(typePromises).then(results => {
     console.log(results)
     expect(results).not.to.contain(false);
 }).then(done, done);

Alternatively, if you're using Mocha, you can actually return the Promise in your it call, and Mocha will handle it all for you:

it('retrieves all results', () => {
 return Promise.all(typePromises).then(results => {
     console.log(results)
     expect(results).not.to.contain(false);
 });
});

I'll close this, because I'm pretty certain the above examples will fix your problems. If they don't, or you have any more questions, feel free to comment and we can continue discussing this. Have a great day 😄 👍

@DannyBrown-xx
Copy link
Author

Thanks for the help @lucasfcosta and @keithamus.

@keithamus your solution worked exactly as described, and I also feel like I understand chai more! Excellent community support. Cheers.

@keithamus
Copy link
Member

😻

@random82
Copy link

Hi,

I have a similar issue:

it("Should return four items", function(done) {
            witService.getCompletedWorkItems().then(result => {
                expect(2).to.eq(1, "2 has to be 1");
            }).then(() => {
                console.log("Pass");
                done();
            }, error => {
                console.log(error.message);
                done(error);
            });
            getCompletedWorkItemRefsDeffered.resolve(responseRefs);
            getWorkItemsDeffered.resolve(responseWIs);
        });

The console output says "Fail" which is correct but the test reported marks this test as passed:

PhantomJS 2.1.1 (Windows 8 0.0.0) LOG: '2 has to be 1: expected 2 to equal 1'
    Takt time calculation for work items list
      √ Should return four items <- should fail
      √ Should return completed items by completion date
      √ Should return takt times as differences between subsequent completion days

@random82
Copy link

random82 commented Feb 18, 2017

Found the solution, change done() to fail() in the second callback. All good now.

it("Should return four items", function(done) {
  witService.getCompletedWorkItems().then(result => {
    expect(2).to.eq(1, "2 has to be 1");
  }).then(() => {
    console.log("Pass");
    done();
  }, error => {
    console.log(error.message);
    fail(error);
  });
  getCompletedWorkItemRefsDeffered.resolve(responseRefs);
  getWorkItemsDeffered.resolve(responseWIs);
});

@vieiralucas
Copy link
Member

Hello @random82

I'm glad you manage to find a workaround, but I think you are missing something here (please correct me if am wrong).
It looks like your workaround works because fail is undefined which causes an undefined is not a function error.

Assuming you are using mocha:

What you need to do is pass the error to done so mocha knows your test failed.

it("Should return four items", function(done) {
  witService.getCompletedWorkItems().then(result => {
    expect(2).to.eq(1, "2 has to be 1");
   }).then(() => {
     console.log("Pass");
     done();
   }, error => {
     console.log(error.message);
     done(error);
   });
  getCompletedWorkItemRefsDeffered.resolve(responseRefs);
  getWorkItemsDeffered.resolve(responseWIs);
});

You could also just return the promise to mocha.

it("Should return four items", function() {
  getCompletedWorkItemRefsDeffered.resolve(responseRefs);
  getWorkItemsDeffered.resolve(responseWIs);

  return witService.getCompletedWorkItems().then(result => {
    expect(2).to.eq(1, "2 has to be 1");
  });
  // if /\ this promise rejects, mocha will assume your test fail
});

Thanks 😄

@random82
Copy link

random82 commented Feb 18, 2017

Hi @vieiralucas

Actually, I'm using karma+jasmine+chai and done(error) was not enough to notify that a test was failing. When I pass fail callback it reports failing tests but I'm getting timeout as well.
I'm using karma-mocha-reporter if that makes any difference

The shorthand version of my test looks like:

it("Should return four items", function(done) {
  witService.getCompletedWorkItems().then(function(result) {
    expect(result).to.have.lengthOf(4);
  }).then(done, fail);
  getCompletedWorkItemRefsDeffered.resolve(responseRefs);
  getWorkItemsDeffered.resolve(responseWIs);
});

@vieiralucas
Copy link
Member

@random82 Thanks for enlightening me, I'm not very familiar with jasmine, but it looks to me that how you are doing is the best way to do it. 😄

Sorry for my misunderstanding

@random82
Copy link

Actually the best version I found so far with proper test result being passed and no timeouts is:

it("Should return four items", function(done) {
  witService.getCompletedWorkItems().then(function(result) {
    expect(result).to.have.lengthOf(4);
  }).then(done, error => {
    fail(error);
    done();
  });
  getCompletedWorkItemRefsDeffered.resolve(responseRefs);
  getWorkItemsDeffered.resolve(responseWIs);
});

I'm not completely happy with it but it works and it's readable.

@keithamus
Copy link
Member

@michaelgallaghertw I would avoid using finally to call done. This would make tests pass, even if the expect() throws an error, as finally is given no arguments. .then(done, done) correctly passes the error back to mocha.

@ghost
Copy link

ghost commented May 20, 2020

Hi ,
I am using mocha steps and when i used above solution i am getting below error:
TypeError: (0 , _mochaSteps.step)(...).then is not a function

This is my code
step("should be able to login ", async () => {
await page.goto(Config.baseUrl);
await loginpage.login("@gmail.com","12345678");
await page.TakeScreenshot();
const success= await page.isElementVisible(".frontendloginsuccessful");
expect(success).to.be.exist;
}).then(() => {
console.log("Pass");
done();
}, error => {
console.log(error.message);
fail(error);
});

I think ,i am doing wrong as i am new to this promise and chai.mocha.can anyone please help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants