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

fix: Don't include project path with supportFile glob #22222

Merged
merged 10 commits into from Jun 10, 2022

Conversation

tbiethman
Copy link
Contributor

User facing changelog

  • The 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 within C:\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

  1. Place a project using cypress in a local directory containing glob syntax chars: ~/projects (foo)/my{test}project. Feel free to get creative.
  2. Make sure that project doesn't have supportFile: false in its config and has a default support file for its project type (likely cypress/support/e2e.js).
  3. Open the project in Cypress and make sure the config is initialized appropriately (no errors in launchpad, you can open the browser and run tests).

How has the user experience changed?

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [n/a] Has a PR for user-facing changes been opened in cypress-documentation?
  • [n/a] Have API changes been updated in the type definitions?

@tbiethman tbiethman self-assigned this Jun 9, 2022
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jun 9, 2022

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,
Copy link
Contributor Author

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.

Copy link
Contributor

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)
Copy link
Contributor Author

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)

@cypress
Copy link

cypress bot commented Jun 9, 2022



Test summary

37553 0 454 0Flakiness 5


Run details

Project cypress
Status Passed
Commit 64d300e
Started Jun 10, 2022 3:19 PM
Ended Jun 10, 2022 3:36 PM
Duration 16:48 💡
OS Linux Debian - 10.11
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

e2e/origin/commands/assertions.cy.ts Flakiness
1 cy.origin assertions > #consoleProps > .should() and .and()
commands/actions/click.cy.js Flakiness
1 ... > scroll-behavior > can scroll to and click elements in html with scroll-behavior: smooth
commands/xhr.cy.js Flakiness
1 src/cy/commands/xhr > abort > aborts xhrs currently in flight
cypress/proxy-logging.cy.ts Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second
2 Proxy Logging > request logging > xhr log has response body/status code when xhr response is logged second

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

@tbiethman tbiethman marked this pull request as ready for review June 9, 2022 15:45
@tbiethman tbiethman requested review from a team as code owners June 9, 2022 15:45
@tbiethman tbiethman requested review from jennifer-shehane and removed request for a team June 9, 2022 15:45
Copy link
Contributor

@ZachJW34 ZachJW34 left a 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!

system-tests/test/config_spec.js Show resolved Hide resolved
@lmiller1990
Copy link
Contributor

lmiller1990 commented Jun 10, 2022

Path is ~/code/f]oo(bar )/cypress10/packages/launchpad. I got this error, but I tried setting supportFile: false, and it still happens. I guess we have this same problem somewhere else?

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
Terminal: git bash

image

@lmiller1990 lmiller1990 self-requested a review June 10, 2022 07:58
@@ -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,
Copy link
Contributor

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.

@tester-at-bmi
Copy link

tester-at-bmi commented Jun 13, 2022

@lmiller1990 i get the same error on 10.1.0 currently by just running npx cypress open.

Windows 11
Cypress v10.1.0

$ npm run cy:open

> cy:open
> npx cypress open --e2e

It looks like this is your first time using Cypress: 10.1.0

[STARTED] Task without title.
[TITLE]  Verified Cypress!       C:\Users\John Doe\AppData\Local\Cypress\Cache\10.1.0\Cypress
[SUCCESS]  Verified Cypress!       C:\Users\John Doe\AppData\Local\Cypress\Cache\10.1.0\Cypress

Opening Cypress...

node:internal/modules/cjs/loader:936
  throw err;
  ^

Error: Cannot find module 'C:\Users\John'
Require stack:
- internal/preload
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at Module._preloadModules (node:internal/modules/cjs/loader:1278:12)
    at loadPreloadModules (node:internal/bootstrap/pre_execution:474:5)
    at prepareMainThreadExecution (node:internal/bootstrap/pre_execution:77:3)
    at node:internal/main/run_main_module:7:1 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ 'internal/preload' ]
}

So currently unable to run cypress tests in the console or in open mode.

@tbiethman
Copy link
Contributor Author

@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?

@diogosantana2011
Copy link

diogosantana2011 commented Jun 13, 2022 via email

@tbiethman
Copy link
Contributor Author

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.

@tester-at-bmi
Copy link

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 Cypress 10.1.0 and rolled back to 10.0.3 for the time being.

BeijiYang pushed a commit to BeijiYang/cypress that referenced this pull request Jun 23, 2022
* 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>
@qaautomationengr
Copy link

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'
Require stack:

  • /var/lib/jenkins/workspace/PluginsUI/cypress/plugins/index.js
  • /var/lib/jenkins/workspace/PluginsUI/cypress.config.js
  • /var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_require_async_child.js
  • /var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/require_async_child.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:956:15)
    at Function.Module._load (node:internal/modules/cjs/loader:804:27)
    at Module.require (node:internal/modules/cjs/loader:1028:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object. (/var/lib/jenkins/workspace/PluginsUI/cypress/plugins/index.js:26:16)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at Module.require (node:internal/modules/cjs/loader:1028:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at setupNodeEvents (/var/lib/jenkins/workspace/PluginsUI/cypress.config.js:11:14)
    at /var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_plugins.js:118:14
    at tryCatcher (/var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/bluebird/js/release/method.js:39:29)
    at RunPlugins.load (/var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_plugins.js:115:9)
    at RunPlugins.runSetupNodeEvents (/var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_plugins.js:276:10)
    at EventEmitter. (/var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/child/run_require_async_child.js:185:22)
    at EventEmitter.emit (node:events:513:28)
    at process. (/var/lib/jenkins/.cache/Cypress/12.4.1/Cypress/resources/app/node_modules/@packages/server/lib/plugins/util.js:33:22)
    at process.emit (node:events:513:28)
    at emit (node:internal/child_process:939:14)`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants