-
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
fix: Don't include project path with supportFile glob #22222
Conversation
Thanks for taking the time to open a PR!
|
@@ -351,7 +351,6 @@ const resolvedOptions: Array<ResolvedConfigOption> = [ | |||
name: 'supportFile', | |||
defaultValue: (options: Record<string, any> = {}) => options.testingType === 'component' ? 'cypress/support/component.{js,jsx,ts,tsx}' : 'cypress/support/e2e.{js,jsx,ts,tsx}', | |||
validation: validate.isStringOrFalse, | |||
isFolder: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was kinda weird. supportFile
was always a file, not a folder, but pre-10.0 we benefited from having the absolute path pre-generated for us prior to resolving the module.
I removed this because 1. it's not a folder and 2. we don't want the absolute path to be appended unknowingly to a glob pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It used to be called supportFolder
a long time ago, maybe hold over from then.
@@ -419,7 +419,7 @@ export async function setSupportFileAndFolder (obj) { | |||
|
|||
const ctx = getCtx() | |||
|
|||
const supportFilesByGlob = await ctx.file.getFilesByGlob(obj.projectRoot, obj.supportFile, { absolute: false }) | |||
const supportFilesByGlob = await ctx.file.getFilesByGlob(obj.projectRoot, obj.supportFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were getting an absolute path prior to this change because obj.supportFile
was an absolute path. Now that it's not an absolute path, we need to ensure absolute paths are returned by the results (absolute
is true by default in getFilesByGlob
)
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking comment. Tested, works great!
Path is Edit happening on develop, too. Edit I tried install 10.0.3 and modifying the binary, seems fine - I think this only happens when using the monorepo in dev mode. So, you fixed the production bug - the dev-only one isn't a big problem, at least, it's not a priority. OS: windows 10 |
@@ -351,7 +351,6 @@ const resolvedOptions: Array<ResolvedConfigOption> = [ | |||
name: 'supportFile', | |||
defaultValue: (options: Record<string, any> = {}) => options.testingType === 'component' ? 'cypress/support/component.{js,jsx,ts,tsx}' : 'cypress/support/e2e.{js,jsx,ts,tsx}', | |||
validation: validate.isStringOrFalse, | |||
isFolder: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It used to be called supportFolder
a long time ago, maybe hold over from then.
@lmiller1990 i get the same error on Windows 11
So currently unable to run cypress tests in the console or in open mode. |
@tester-at-bmi Thank you for the report. Can you log an issue with more details about your environment/Cypress config so we can investigate further? |
Hi,
Good afternoon!
I’ve added the configuration file on the comment for description and logs.
When you ask for environment information, what do you mean?
Jenkins is set up on an Windows Operating System running on Virtual Machine where the job is running.
Locally on my Mac all spec are runnable and configuration picks up the spec file, but not in Jenkins.
There was a similar issue (#22040) to this regarding supportFile which was included in release 10.1.1.
Let me know if I can provide anything further,
Thank you,
Best regards,
Diogo Santana | QA Automation Engineer | Games Innovation Labs
[signature_1506923581]
From: Tyler Biethman ***@***.***>
Date: Monday, 13 June 2022 at 15:48
To: cypress-io/cypress ***@***.***>
Cc: Diogo Santana ***@***.***>, Manual ***@***.***>
Subject: Re: [cypress-io/cypress] fix: Don't include project path with supportFile glob (PR #22222)
@tester-at-bmi<https://github.com/tester-at-bmi> Thank you for the report. Can you log an issue with more details about your environment/Cypress config so we can investigate further?
—
Reply to this email directly, view it on GitHub<#22222 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AN7COBZGUEHPDZ3DIWPLYRLVO433PANCNFSM5YKOLRRQ>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
Sensitivity: Internal
Sensitivity: Internal
|
Hey @diogosantana2011, I think you have plenty of info on #22272. My comment was for @tester-at-bmi, who reported seeing another issue here in his above comment. Sorry for any confusion, there are a lot of notifications going around for these linked issues. |
Yes sorry for hijacking this thread/topic. I will dig true the current reports on GH and see if it's already reported, if not i will create a new report. Basically i'm not able to use the latest |
* fix: Don't glob project path in supportFile lookup * Updating unit tests around supportFile 'isFolder' * Adding unit test to validate the projectRoot isn't globbed * Adding system test to validate successful run * This is more accurate * Updating snapshot to reflect now missing absolute path from the supportFile value * Adding e2 launchpad test Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
I got an error below while running jenkins in linux `Your configFile threw an error from: /var/lib/jenkins/workspace/PluginsUI/cypress.config.js The error was thrown while executing your e2e.setupNodeEvents() function: Error: Cannot find module 'node-xlsx'
|
User facing changelog
supportFile
can now be detected within projects that contain glob syntax characters in their absolute paths.Additional details
We glob the
supportFile
config value to determine whether the file exists during project setup. We were using the supportFile's absolute path as the glob pattern, which could fail the lookup if the absolute path contained glob syntax:()
,[]
,{}
,*
,?
, etc. Some of these are more common than others in certain OSs. The issue was originally logged due to a lookup failure when the project was located withinC:\Program Files (x86)\...
on Windows, but it can be reproduced on Mac/Linux by placing the project in a directory containing these characters:~/projects (foo)/test-project
I took a look at why the absolute path was being used here, and I think it was a holdover from the previous non-glob implementation that directly used module resolution. I updated the glob lookup to no longer include the projectRoot, which corrects this issue.
Steps to test
~/projects (foo)/my{test}project
. Feel free to get creative.supportFile: false
in its config and has a default support file for its project type (likelycypress/support/e2e.js
).How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?