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

Using migration tool from 9.7.0 -> 12.6.0 does not convert testFiles to specPattern correctly #25947

Closed
monkpit opened this issue Feb 24, 2023 · 13 comments · Fixed by #25969
Closed

Comments

@monkpit
Copy link

monkpit commented Feb 24, 2023

Current behavior

My original cypress.json contained testFiles: ['array', 'of', 'globs'].

During the guided migration, this was converted to one big string, but was done incorrectly: specPattern: 'cypress/e2e/array,of,globs' - cypress/e2e is only added to the first glob, and the rest are tacked on with comma separators.

This causes 2 problems.

  1. No specs are found when Cypress starts because it doesn't parse the string as multiple globs, it seems to treat it as 1 big glob.
  2. Even if it did work this way, cypress/e2e/... path prefix was only added to the first path element, so only 1 out of 3 globs would return spec files.

Desired behavior

testFiles: [ 'array', 'of', 'globs' ] should convert to specPattern: [ 'cypress/e2e/array', 'cypress/e2e/of', 'cypress/e2e/globs' ] - which seems to be the only way it works correctly (for me, anyways).

Test code to reproduce

Convert a project with cypress.json that has a testFiles value that contains an array of glob patterns.

Cypress Version

12.6.0

Node version

16 LTS

Operating System

MacOS 12.6

Debug Logs

No response

Other

No response

@monkpit
Copy link
Author

monkpit commented Feb 24, 2023

Another weird thing that happened during the conversion was that none of my specs were renamed *.cy.js, they remained *.spec.js - I can file another issue if needed, but wasn't sure if it could somehow be related.

Another note - Coming from v9 with the integrationFolder config, it seems strange that cypress/e2e is required here - I would have expected some configs like e2eFolder and componentFolder to exist, but it seems like they don't.

@lmiller1990
Copy link
Contributor

Uh oh, sounds like the logic converting the array to a glob didn't work correctly here. Luckily you were able to fix in manually, but still worth fixing on our end.

none of my specs were renamed *.cy.js, they remained *.spec.js

This seems weird - all the repos I tested did this correctly. Can you share a minimal reproduction (maybe add cypress.json with your testFiles array, any other customizations you've got, and a spec or two so we can reproduce it)?

@monkpit
Copy link
Author

monkpit commented Feb 27, 2023

I'll see if I can narrow down the cause with a minimal example and share that.

@monkpit
Copy link
Author

monkpit commented Feb 27, 2023

https://github.com/monkpit/testFiles-migration-cypress-error

Note: I'm using yarn but I don't think it should have any effect on reproducing the issue here.

Steps to reproduce after cloning the project:

  • run yarn to install dependencies with Cypress@9.7.0
  • run yarn cypress open to verify that the testFiles array is working - we should only see specs in the 2-advanced-examples folder, matching a*.spec.js and w*.spec.js
  • run yarn add cypress@12.6.0 to upgrade cypress
  • run yarn cypress open again to trigger the migration process
  • ⚠️ Notice: the specs do not get renamed as *.cy.js here
  • 💣 The error occurs on the last step when migrating the config - specPattern contains a malformed string even in the Migration tool UI
  • Finish the migration and open the e2e suite (in Chrome or whatever)
  • 💣 No specs are available to run

@lmiller1990 I hope this will suffice, please let me know if you are not able to reproduce the issue. Thanks!

EDIT: If it matters - I ran this on macOS 12.6 and node 16.19.0

@lmiller1990
Copy link
Contributor

lmiller1990 commented Feb 28, 2023

Right, I reproduced the issue. I think the reason we don't change anything is because if users have a custom testFiles, we don't necessarily know what extension they are using - it could be testFiles: "**/*.test.js", or something other than the default.

Either way, we should handle this properly. I'll take a look at this one today.

@monkpit
Copy link
Author

monkpit commented Feb 28, 2023 via email

@lmiller1990
Copy link
Contributor

I'll build you a pre-release of the fix #25969 that you can test early, give me an hour or two, thanks!

@lmiller1990
Copy link
Contributor

Hi @monkpit, want to give this pre-release a try with the fix? Linux + MacOS builds: e02dff7#comments

@lmiller1990
Copy link
Contributor

Fixed, see #25969 for screenshots.

@monkpit
Copy link
Author

monkpit commented Feb 28, 2023 via email

@monkpit
Copy link
Author

monkpit commented Feb 28, 2023

@lmiller1990 looks great! Thank you for the quick fix. I tested it on the repo with the original issue and it is resolved with the pre-release build.

@lmiller1990
Copy link
Contributor

Great! This will be in the next Cypress release (that's every 2 weeks). But since you've successfully migrated, you should be good to just go with Cypress 12.7 for now.

If you have any feedback on Cypress 12 vs Cypress 9 in general, please tell me and I will forward it to the right people.

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 14, 2023

Released in 12.8.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.8.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants