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

Assertion failure in async test case does not fail test overall #4742

Open
MadLittleMods opened this issue Jul 17, 2019 · 54 comments
Open

Assertion failure in async test case does not fail test overall #4742

MadLittleMods opened this issue Jul 17, 2019 · 54 comments
Labels
prevent-stale mark an issue so it is ignored by stale[bot] type: bug

Comments

@MadLittleMods
Copy link

MadLittleMods commented Jul 17, 2019

Related issues

Current behavior:

The assertion fails(in red) but the overall test still passes,

Desired behavior:

Asserts with cy work in async test case

Steps to reproduce: (app code and test code)

Use the following testing code with async and notice the test succeeds.

https://github.com/MadLittleMods/cypress-test-tiny/pull/1/files

const generateFixtures = function() {
  return new Promise(resolve => {
    setTimeout(() => {
      resolve("bar");
    }, 1000);
  });
};

describe("Some page", function() {
  it("shows something", async function() {
    const fixtures = await generateFixtures();

    cy.visit(`http://google.com/?foo=${fixtures}`);

    cy.contains("somethingthatodoesNOTexist");
  });
});

Versions

  • Cypress 3.4.0
  • Node v10.15.1
  • Windows 10
@MadLittleMods MadLittleMods changed the title Assertion failure in async test case does not fail Assertion failure in async test case does not fail test overall Jul 17, 2019
@jennifer-shehane
Copy link
Member

@MadLittleMods The example above is missing information to run successfully, notably the apiBaseUrl and jQuery dep.

Please provide a completely reproducible example that we can run that shows this bug and we will reopen the issue.

@MadLittleMods
Copy link
Author

MadLittleMods commented Jul 22, 2019

@jennifer-shehane Here is a barebones test case that demonstrates the problem, https://github.com/MadLittleMods/cypress-test-tiny/pull/1/files

MadLittleMods added a commit to MadLittleMods/cypress-test-tiny that referenced this issue Jul 22, 2019
@jennifer-shehane
Copy link
Member

Thanks for providing a reproducible example!

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label Jul 23, 2019
@jennifer-shehane jennifer-shehane added type: bug stage: ready for work The issue is reproducible and in scope and removed stage: ready for work The issue is reproducible and in scope labels Jul 23, 2019
@drFabio
Copy link

drFabio commented Aug 16, 2019

Not sure if related but this test is also failling with native promises mixed with cypress promises with this code:

it('It fails on promise but it passes', () => {
  new Cypress.Promise((resolve, reject) => {
    Promise.reject(new Error('Error from native promise')).catch(err => {
      reject(err);
      expect(true).to.be.false;
    });
  });
});

It produces:
Screenshot 2019-08-16 at 12 10 20

it also passes while failing on:

it('It fails on promise but it passes', () => {
  new Cypress.Promise((resolve, reject) => {
    Promise.reject(new Error('Error from native promise')).catch(err => {
      reject(err);
    });
  }).catch(err => {
    expect(true).to.be.false;
  });
});

@trucklos
Copy link

Temperorary workaround for me is to wrap the promise in a Cypress command. Note that errors in catch will just time out the test that calls this command. In my case, in my commands js:

Cypress.Commands.add('resetSomeData', function() {
    return new Cypress.Promise((resolve, reject) => {
      graphqlFetchObject
        .request("mutation { doSomething }")
        .catch(function(err) {
          let didDeleteData = false;
          expect(didDeleteData).to.be.true;
          reject(err);
        })
        .then(function(resp) {
          resolve(resp);
        });
    }).then((resp) => {
        expect(resp.didItHappen).to.be.true;
    });
})

@agugut-nexgen
Copy link

agugut-nexgen commented Sep 23, 2019

Temperorary workaround for me is to wrap the promise in a Cypress command. Note that errors in catch will just time out the test that calls this command. In my case, in my commands js:

Cypress.Commands.add('resetSomeData', function() {
    return new Cypress.Promise((resolve, reject) => {
      graphqlFetchObject
        .request("mutation { doSomething }")
        .catch(function(err) {
          let didDeleteData = false;
          expect(didDeleteData).to.be.true;
          reject(err);
        })
        .then(function(resp) {
          resolve(resp);
        });
    }).then((resp) => {
        expect(resp.didItHappen).to.be.true;
    });
})

Hi,
I've the same problem
Can you help me to apply the worarround?

@MadLittleMods MadLittleMods mentioned this issue Sep 27, 2019
6 tasks
@kuceb
Copy link
Contributor

kuceb commented Sep 27, 2019

So this issue is due a bug where Cypress doesn't support async/await tests bodies or tests that return promises. This wouldn't be too hard to fix, but we need to prioritize it. We'll need to rangle all the issues that are affected by this

@kuceb
Copy link
Contributor

kuceb commented Sep 27, 2019

@ryan-snyder its a cypress issue. Mocha supports this, but we do some nasty hacks on top of mocha and likely didn't wire something up properly, such as awaiting a promise if it's returned.

@kuceb
Copy link
Contributor

kuceb commented Sep 27, 2019

I'll likely spike into fixing this, possibly it will be simple. If not I'll work on getting it prioritized.

@brian-mann
Copy link
Member

brian-mann commented Sep 27, 2019

@bkucera and I have been over this a bunch of times before and it's something on our end that we need to fix. It has to do with the interop between promises being returned to the test, and Cypress also knowing that commands were enqueued.

We can generally always figure out how to do the right thing, but it will involve doing things like adding a .then() and additionally monkey patching Promise.prototype.then to know when the promise chain is settled, and to know whether or not cypress commands were enqueued within each .then() block.

We need to do things like intelligently throw if you've enqueued cypress commands inside of a native promise .then() but not returned cy, or we need to automatically figure this out and prevent the .then() from resolving until the cypress command chain has resolved inside of there. I'm in favor of the latter.

@agugut-nexgen

This comment has been minimized.

@damaon
Copy link

damaon commented Nov 13, 2019

What is the point of having testing tool that doesn't even assert properly? This should have P1

Can someone guide me to related code?

@nabilamin
Copy link

nabilamin commented Nov 13, 2019

Is there a workaround / solution for this issue yet? Experiencing the same problem.

@nabilamin
Copy link

nabilamin commented Nov 13, 2019

@ryan-snyder Was able to solve my problem with a cy.wrap() ...
looks something like this

cy.wrap(
   AWS.listAttachedRolePolicies('Admin').then((list) => {
         expect(5).to.be.null;
   })
);

@amiramix
Copy link

amiramix commented Dec 6, 2019

Could issue #1417 be related ?

@swathisham

This comment has been minimized.

@swathisham
Copy link

swathisham commented Dec 28, 2019

@jennifer-shehane @brian-mann I am getting the same issue.. assertion fails but test shows as PASSED.
image
image

image

@agugut-nexgen
Copy link

Hello I have the same problem with a test that compare 2 array content with deep equal.
Any news about the topic??

@ppunjjs
Copy link

ppunjjs commented Jan 29, 2020

Any update on this issue? My test assertion is failed but test status is still green.

@jennifer-shehane
Copy link
Member

Please refrain from commenting asking for updates 🙏

This issue is still in the 'ready for work' stage, which means no work has been done on this issue as of today, so we do not have an estimate on when this will be delivered.

If there are any updates the status label on the issue will change, a linked PR will be opened, or a comment will be made by someone on the Cypress team. Otherwise, there are no updates.

@BiosBoy
Copy link

BiosBoy commented May 10, 2020

The same story here.

@jennifer-shehane btw why we should not ping you guys? This is a major issue inside Cypress, caused a lot of people. So that's the only way to say you "Hey, we're still here and need help". As a result, make some impact on reordering among your roadmap prioritizations. Based on how much folks ask for. :(

Otherwise, it stuck in your backlog for a while...

Thanks.

@agugut-nexgen
Copy link

Hello
I solved used cy.wrap instead assert
Also transform some assertion with promises in cypress tasks

@Sathish787
Copy link

Sathish787 commented May 22, 2020

Guys, I faced same issue, BDD assertions fails but test passes.
Use await before async function or line of code:

For example: I used jquery inside BDD assertions
[reason want to use jquery to avoid log messages in test runner. In this way, it takes only 20 secs to execute test whereas If I do cypress way assertion, it was taking more than 50 secs coz need to validate css properties of each cell value in ag-grid table],

assert fails not matching [1,2,1] but test was passing. No success with try and catch. I tried with await like below and finally test fails.

it.only("Sample Test", async () => {
... line of codes ...
await cy.get(".ag-header-container .ag-header-row").then((element) => {
          {few assertions}
          .....
            cy.wrap(element).find(".ag-cell-label-container").each(elem => {        
                expect([
                    Cypress.$(elem).find(".ag-sort-ascending-icon").length,
                    Cypress.$(elem).find(".ag-sort-descending-icon").length,
                    Cypress.$(elem).find(".ag-sort-none-icon").length
                ]).to.deep.equal([1,2,1])                
            })
        })
})

@thril
Copy link

thril commented May 23, 2020

I didn't think that the cy functions returned promises that may be awaited. Perhaps the await is working, but that code is misleading? I'm curious if adding a cy.wait(0) right before the cy.get has any impact, even if it's not the "correct" solution.

@jennifer-shehane
Copy link
Member

This is still behaving incorrectly in 5.0.0

Repro

const getBar = () => {
  return new Promise(resolve => {
  setTimeout(() => {
    resolve('bar')
  }, 1000)
});
};

it('test', async () => {
  cy.contains(await getBar())
})

@Sathish787
Copy link

Sathish787 commented Aug 28, 2020

After spending lot of time in Cypress, I came out with solutions which helped me in writing good testcases and hope same would be for others too.

  1. Don't use async in testcase like I specified in my previous post. It affects lot of areas like in assertion (doesn't assert properly sometimes) and also if you check mochawesome report it doesn't show your line of codes rather shows some other promise return function which I felt like not convincing.

Always stick to :

it("sample test", () {
    })         
    
(not)

it("sample test", async() {
    })   
  1. Try to avoid using async functions in cypress. I replaced all my functions asynchronous to synchronous functions and it's works perfectly and same result all time. Only place I used async function where I need to do recursive to get all elements text until given column value in dynamic loading table.

Async & Sync function in customFunctions.js file

export class customFunctions {

Asynchronous function: (iter is for terminating infinite loop if text does not exist)
    async scrollhztnlAndGetAllElements(exp_value, webElement, elemlist = [], iter = 0) {
        iter++
        if (iter == 100)
            return cy.wrap([], { log: false })
        let vlist = await cy.get(webElement, { timeout: 10000, log: false }).text({ log: false }).promisify()
        elemlist = elemlist.concat(vlist)
        if (elemlist.includes(elemlist.find((value) => value.includes(exp_value)))) {
            return cy.wrap(Cypress._.uniq(elemlist), { log: false })
        } else {
            cy.get(webElement, { log: false }).last({ log: false }).click({ waitForAnimations: false, log: false })
            return customFunct.scrollhztnlAndGetAllElements(exp_value, webElement, elemlist, iter)
        }
    }
 Synchronous function:
  replaceCharInArray(exp_array, emp_arr = []) {
        exp_array.forEach(evalue => {
            let value = evalue.replace("(mm)", "").replace("(000s)", "").trim()
            emp_arr.push(value)
        })
        return emp_arr
    }

}
export const customFunct = new customFunctions() 

And in testcase, I import class file and called both async function (scrollhztnlAndGetAllElements) and sync functions (replaceCharInArray) as below shown. Synchronous function doesn't need to be wrapped and only async function need to be wrapped as in below sample code.

In testcase file:
import { customFunct } from '../../support/customFunction'

it("Column Header Test", () => { 
     cy.wrap(customFunct.scrollhztnlAndGetAllElements(Cypress._.last(expectedHeaders), locator.tableColumnHeader))
              .then(actualHeaders => {
                     expect(customFunct.replaceCharInArray(actualHeaders)).to.deep.equal(expectedHeaders)  
                     expect(Cypress._.join(actualHeaders)).to.not.include("undefined")
    })
})

(OR) If it is single assertion, i would have done like this for values from async function:

it("Column Header Test", () => { 
     cy.wrap(customFunct.scrollhztnlAndGetAllElements(Cypress._.last(expectedHeaders), locator.tableColumnHeader))     
          .should("to.deep.equal", expectedHeaders)                       
 })

If you want to get single element value, then use Jquery inside thenable or each:

cy.get(locator.tablerowlist).each((elem, indx) => {
       let nameColumnValue = Cypress.$(elem[indx]).text()
      ....
     ....
      rest lines of codes
})
  1. If you want to do multiple assertion for list of elements then you can use similar to below function so that execution would be faster than using cy.get(locator).each(elem=> {})
sortCheckElements(elements, len = 0) {
        for (var i = 0; i < Cypress.$(elements).length; i++) {
            if (Cypress.$(elements[i]).has(locator.sortAscIcon).length > 0 &&
                Cypress.$(elements[i]).has(locator.sortDescIcon).length > 0 &&
                Cypress.$(elements[i]).has(locator.sortNoneIcon).length > 0) {
                len = len + 1
            }
        }
        if (len === Cypress.$(elements).length)
            return true
        else
            return false
    }

hgHeaderCssPropertyValidation(elements, len = 0) {
        for (var i = 0; i < Cypress.$(elements).length; i++) {
            if (Cypress.$(elements[i]).css("font-size") === "11.5px" &&
                Cypress.$(elements[i]).css("font-weight") === "700" &&
                Cypress.$(elements[i]).css("color") === "rgb(255, 255, 255)" &&
                Cypress.$(elements[i]).css("text-overflow") === "clip" &&
                Cypress.$(elements[i]).css("white-space") === "normal" &&
                Cypress.$(elements[i]).css("text-align") === "center") {
                len = len + 1
            }
        }
        if (len === Cypress.$(elements).length)
            return true
        else
            return false
    }

In testcase, use:

cy.get(locator).then(elem => { expect(customFunct.sortCheckElements(elem)).to.be.true })

cy.get(locator).then(elem => { expect(customFunct.hgHeaderCssPropertyValidation(elem)).to.be.true })
  1. If you want to assert list of values. then install cypress-commands library (here I use to('array')) from Cypress plugin page.
cy.get(locator.tableColumnHeader).text().to("array")
      .should("to.deep.equal", ["Time", "Legacy (mm)", "Standard Origin (mm)"])
  1. Likewise, instead of using cy.fixture and thenable. I used import fixture file and assigned to a variable and then I use variable in all testcases.
In testcase file:
const drillData = require('../../fixtures/drillData.json')

it("some test" () => {
   cy.server()
   cy.route("POST", APIEndPoint, drillData).as("dataDrill")
})
  1. You could place expected values into json file in fixture and could use in testcase:
In UserInfo json file:
{
"expectedCusInfo": [{"Region", "Time Period", "XYZ", "ABC"],
"expectedMessage": "User has been added successfully"
}

In testcase file:

const userData= require('../../fixtures/UserInfo.json')

it("some test" () => {
  cy.get(locator).text().to("array").should("be.deep.equal", userData.expectedCusInfo)
  cy.get(locator).should("have.text", userData.expectedMessage)
})
  1. Also, instead of using UI elements to make cypress to wait use api call. Example, for page to load, instead of waiting for progress bar to disappear. I followed to wait for api call to finish, here api is not mocked by data still it hits application server.
it("some test", () => {
  cy.server()
  cy.route("GET", ApiEndPoint).as("customerData")
  cy.visit("/customerpage")
  cy.wait("@customerData")
})

Hope, this would be helpful. Object chaining better than using async in testcase to get reliable results all time.

@feliperaul
Copy link

This is still a bug in 8.6.0

@utiq
Copy link

utiq commented Oct 29, 2021

This is still a bug in 8.6.0

I can confirm. I'm using version 8.7.0

@ranatoqeer807
Copy link

ranatoqeer807 commented Dec 1, 2021

How we can identify a test case failed in Cli if assertion failed as cypress passes the test cases even after assertion failed
Using version: 8.7.0

image

@swinejelly
Copy link

I've just confirmed that this is an issue in version 9.5.0. I accidentally left an async on my test function that I didn't really need and spent much of this afternoon trying to debug why my tests kept passing even when I had obviously broken assertions in them. After finding this I had to go through our existing test cases to fix other async tests that would succeed no matter what.

We are fairly new to Cypress but the fact that this issue has been open for 3.5 years is making me reconsider whether Cypress is reliable enough for us to be using.

@swinejelly
Copy link

I was able to set up a rough eslint error for this using the eslint-plugin-regex package as follows for anyone else who needs it:

{
      "plugins": [
        "regex"
      ],
      "files": ["**/*.cy-spec.ts"],
      "rules": {
        "regex/invalid": [
          "error", [{
            "regex": "\\sasync\\s",
            "message": "Do not use async functions for Cypress tests. They will succeed even if Cypress assertions therein fail: https://github.com/cypress-io/cypress/issues/4742",
            "replacement": " "
          }]
        ]
      }
}

But it's very important for a testing framework to correctly fail when its assertions aren't met; relying on developers to fully follow Cypress's issue tracker, set up custom lint errors, follow blog posts, etc. to be aware of this issues and its workarounds will inevitably mean a lot of developers won't understand this and will write defective tests, leading to defective products shipping.

Is there a way this can be treated as a P1, or are there any blockers/resourcing problems that we could help with?

@bahmutov
Copy link
Contributor

bahmutov commented Mar 8, 2022

@swinejelly you can use the official Cypress ESLint plugin to catch this already https://github.com/cypress-io/eslint-plugin-cypress#rules

@swinejelly
Copy link

@bahmutov Thank you, I wound up installing the Cypress ESLint plugin instead.

Do you mind looking at this issue I filed in the Cypress ESLint repo please?

@bahmutov
Copy link
Contributor

bahmutov commented Mar 8, 2022 via email

@cypress-bot cypress-bot bot added stage: backlog and removed stage: ready for work The issue is reproducible and in scope labels Apr 29, 2022
@AnatoliiOlshevskyiBlackBird

Any updates on this one? Have the same issue.

@AnatoliiOlshevskyiBlackBird
Copy link

AnatoliiOlshevskyiBlackBird commented Sep 8, 2022

cy.wrap

@agugut Could u please share the example?

@bahmutov
Copy link
Contributor

bahmutov commented Sep 9, 2022

I wrote a plugin https://github.com/bahmutov/cypress-catch-async-tests that catches this, for anyone interested.

error

@awoerp
Copy link

awoerp commented Oct 25, 2022

I am sure that this won't be helpful to everyone but I found a work around for my use case which is conditionally running tests depending on the results of an asynchronous query of a remote database. My solution was to remove any asynchronous stuff from the tests and instead move it into the before() function which runs before all of the tests for that specific spec file.

describe('customer-setup', async () => {
    beforeEach(() => {
        checkIfLoginNecessary()
        cy.visit("/customer")
    });

    before(async () => {
        customerData1List = await db.getEntityByQuery<Customer>(
            'customer',
            'name',
            CustomerData1.name
        )
    })

    it('can create customer without location', () => {

        console.log(customerData1List)

        if (customerData1List.length === 0) {
            cy.get(Buttons.browse_customers_new_customer).click()
            cy.get(Forms.create_customer_dialog_name).click()
                .type(CustomerData1.name).tab()
                .type(CustomerData1.websiteUrl).tab()
                .type(CustomerData1.phoneNumber)
            cy.get(Buttons.create_customer_dialog_submit).click()
            cy.contains(CustomerData1.name)
            cy.contains(CustomerData1.displayedPhoneNumber)

        } else {
            console.log("skipping")
        }
    })

});

@BlueWinds
Copy link
Contributor

BlueWinds commented Feb 16, 2023

I'm looking into a fix for this issue; it's required work on the way to #1417.

An example to demonstrate the most basic case:

it('passes', async () => {
  cy.wrap(true).should('eq', false)
})

The promise resolves immediately, but there are still items in the Cypress command queue. We should clearly be waiting for both the test function to resolve and the command queue to be empty.

Notably, it works if you wrap it in cy.then():

it('fails, as it should', () => {
  cy.then(async () => {
    cy.wrap(true).should('eq', false)
    await 1
  })
})

meaning that Cypress has already 'solved the hard part' - we know how to do the right thing (because the right thing happens when we wrap our original function, unchanged, in cy.then()), we're just not doing it when the top-level function is async / returns a promise.

@nagash77 nagash77 added the prevent-stale mark an issue so it is ignored by stale[bot] label Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prevent-stale mark an issue so it is ignored by stale[bot] type: bug
Projects
None yet
Development

No branches or pull requests