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

chore(deps): revert examples/yarn-modern-pnp to cypress 12.9.0 #904

Closed

Conversation

MikeMcC399
Copy link
Collaborator

@MikeMcC399 MikeMcC399 commented May 8, 2023

This PR reverts examples/yarn-modern-pnp from Cypress 12.11.0 to Cypress 12.9.0.

Issue

Changes introduced in Cypress 12.10.0 have caused the Cypress App to fail to work when invoked through yarn cypress open in a Yarn Plug'n'Play environment using Cypress 12.10.0 or 12.11.0

To reproduce, execute locally in master branch:

cd examples/yarn-modern-pnp
yarn
yarn cypress open

Click on "E2E Testing"
Note error message relating to "bluebird" (details differ according to operating system - see cypress-io/cypress#26567 (comment)). For Ubuntu 22.04 the error looks like this:

image

Verification

After application of this PR, execute locally:

cd examples/yarn-modern-pnp
yarn
yarn cypress open

Click on "E2E Testing"
Select Electron
Select Start E2E Testing in Electron
Select spec.cy.js
Confirm successful execution of Cypress test

Related issues

@MikeMcC399 MikeMcC399 marked this pull request as ready for review May 8, 2023 07:13
@nagash77
Copy link
Contributor

nagash77 commented May 8, 2023

I feel like we should fix the test example rather than have folks use an older version of Cypress.

@MikeMcC399
Copy link
Collaborator Author

Hi Ben @nagash77

I feel like we should fix the test example rather than have folks use an older version of Cypress.

I don't see any way to fix the example directly, because the problem is in Cypress, not in the example. So the fix needs to come from Cypress.

There is a general compatibility issue between the latest Cypress 12.x versions and Yarn's Plug'n'Play mode which does not use node_modules like npm does, but instead has the dependencies stored in the .yarn directory in a different format.

The example actually runs fine in CI on GitHub. The problem only becomes apparent if a user manually starts the Cypress desktop app locally.

If you want to delay a decision on this whilst a potential Cypress fix is clarified, we can put the PR into Draft status.

@nagash77
Copy link
Contributor

nagash77 commented May 8, 2023

Yeah I agree. sorry I was not clear @MikeMcC399 . I meant I think we need to fix the the problem causing this regression in Cypress.

@MikeMcC399
Copy link
Collaborator Author

I only intended this PR as a temporary mitigation, not as a permanent fix. Please feel free to reject it, postpone it or merge it, as you see fit.

@MikeMcC399 MikeMcC399 force-pushed the yarn-pnp-to-cypress-12-9-0 branch 4 times, most recently from d1d0147 to 6db1eb5 Compare May 12, 2023 13:55
@marktnoonan
Copy link

In the next release of Cypress we are reverting the yarn pnp PR, which I think will make this change obsolete? Or at least, will allow us to update the example to match the latest Cypress release again.

@MikeMcC399
Copy link
Collaborator Author

@marktnoonan

In the next release of Cypress we are reverting the yarn pnp PR, which I think will make this change obsolete? Or at least, will allow us to update the example to match the latest Cypress release again.

Let me put the PR into draft state for the moment. If the issue is mitigated in the next Cypress release I will then close it, otherwise I will bring it up again for consideration.

Thanks for reviewing and for your comment!

@PilotConway
Copy link
Contributor

One thing to note for this in my teams experience, the PnP issue only seems to apply when running the UI locally. The action works without issue even on 12.10 and newer since its using cypress run which doesn't use bluebird or any other dependencies that are missing from the package.json files. We have been using the newer versions of cypress since they came out in our action and have had no issues. But locally we are locked to 12.9.0.

@MikeMcC399
Copy link
Collaborator Author

@PilotConway

One thing to note for this in my teams experience, the PnP issue only seems to apply when running the UI locally.

We agree on this (see also cypress-io/cypress#26567). yarn cypress run works fine, also with Cypress 12.12.0. It's only yarn cypress open which fails.

@MikeMcC399
Copy link
Collaborator Author

@MikeMcC399 MikeMcC399 closed this May 24, 2023
@MikeMcC399 MikeMcC399 deleted the yarn-pnp-to-cypress-12-9-0 branch May 24, 2023 06:02
@MikeMcC399
Copy link
Collaborator Author

Problems with using the Cypress desktop app in E2E testing mode together with Yarn Plug'n'Play are resolved in Cypress 12.13.0. Since the example here is only running an E2E test, reverting to a previous version is no longer necessary.

(Yarn Plug'n'Play continues to cause an issue in Component Testing mode where dependencies are not recognized.)

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

Successfully merging this pull request may close these issues.

5 participants