-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
[setupNodeEvents] on("before:spec", ()=>...) doesn't await the promise returned by the event handler #24403
Comments
@ioannisNoukakis, thanks for logging this issue and providing the reproduction! I was able to reproduce the issue. I am going to route this issue to the appropriate team. |
|
@emilyrohrbough, just for your information, as I've seen you previously fixed a similar issue #22360 |
@alexniculae, I believe all versions of Cypress 10 are affected as there were architectural changes that didn't account for the |
Thank you for the details @mschile
|
Please note that, for my org, this is a blocking issue for upgrading Cypress to v10 or above
Dur to this matter we are unable to update Cy from v9 to v10 or 11. Could you please share an ETA for the fix? Thank you, Alex |
@alexniculae Is this a fix you'd be will to contribute to unblock your org? |
@emilyrohrbough, I'd love to, but for full transparency, that is very likely over my level of competences when it comes to coding 🙂 No real rush from my end to update to v10 or 11, just that we're missing out on some nice new features, bug fixes and everything else that is coming with newer versions. |
@alexniculae No problem! |
@emilyrohrbough If no one has started to fix this I'm willing to contribute |
@ioannisNoukakis We would love your contribution! |
@ioannisNoukakis Any update? |
@emilyrohrbough Yeah I thought I had the bandwidth to do this fix but I don't. Sorry |
@ioannisNoukakis no worries! I'm going to pick this up then. Thanks for trying! We love getting contributions 😄 |
Hey @emilyrohrbough, Curious if we have any updates on this one 🙂 Was this picked up as WIP, do we have a proposed release for a fix? Thank you |
@alexniculae I started to work on this and had to put it off for due to some refactoring required in the app for this to work correctly. Hoping to take some time soon to wrap this up. |
@emilyrohrbough - is there any movement expected on this issue in the near future at all? |
@jhulme / @alexniculae I apologize that this hasn't been prioritized. I have been able to spend a few hours on this and have this fixed for Here is the branch with the progress: emily/before-spec-promise If you are interested in collaborating to get this across the line, that would be greatly appreciated! Essentially we need to cancel the first promise and/or correctly respond to & execute the last promise that finalizes. |
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Current behavior
The promise returned by the event handler of
before:spec
is not awaited but is run concurrently, alongside the spec.Desired behavior
As specified in the docs here: https://docs.cypress.io/api/plugins/before-spec-api#Syntax,
before:spec
is supposed to await the completion of the promise returned by it before running the spec.Test code to reproduce
See https://github.com/ioannisNoukakis/cypress_before_spec_promise, contact me if any questions
Cypress Version
10.11.0
Node version
v16.17.0
Operating System
Manjaro 22.0.0 Sikaris - x86_64 Linux 5.15.74-3-MANJARO
Debug Logs
No response
Other
No response
The text was updated successfully, but these errors were encountered: