fix: CLI filtering --spec when commas in glob pattern

[hang-up]

• Closes CLI filtering --spec with glob pattern does not find spec files #16102

User facing changelog

Enable comma separated glob pattern matching for CLI parameter spec. Fixes #16102

Additional details

This PR aims at fixing a bug discovered since v6.5.0.
When glob patterns were defined with at least one comma, the original implementation would assume that when parsed as a string, the spec argv could be split into an array.

This assumption holds when the argv is a string of spec files to run:

• --spec "cypress/integration/spec-1.spec.js,cypress/integration/spec-2.spec.js" would give an array similar to

[
  "cypress/integration/spec-1.spec.js", 
  "cypress/integration/spec-2.spec.js"
]

Unfortunately in the case of comma separated glob pattern, we would end with erroneous paths:

• --spec 'cypress/integration/{[!fix]*.spec.js,!(_unit)/**/[!fix]*.specs.js}' would give an array similar to

[
  "cypress/integration/{[!fix]*.spec.js", 
  "!(_unit)/**/[!fix]*.specs.js"
]

This PR fixes this issue by performing a glob check before mapping the string to an array of spec paths.

How has the user experience changed?

Following the same file structure as defined in the issue ticket

.
├── cypress
│   ├── integration
│       └── _fixtures
│           └── fixture-should-not-match.spec.js
│       └── _unit
│           └── unit-should-not-match.spec.js                  
│       └── sub-level-1
│           └── should-match-level-1.spec.js                   --> this one should match!
│         └── sub-level-2
│             └── should-match-level-2.spec.js                 --> this one should match!
│       └── fixture-should-not-match.spec.js

Before

After

PR Tasks

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

[cypress-bot]

Thanks for taking the time to open a PR!

• Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
• Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

[CLAassistant]

All committers have signed the CLA.

[hang-up]

I'm just realizing now that some snapshot tests are failing.
I'll await the initial feedback on this PR before taking any steps further (eg. update those snapshots).

[jennifer-shehane]

@hang-up The change has the unintended side effect of updating the value that is printed to CLI when showing what Cypress searched for in the specs. We'd like to maintain the original behavior or printing the original search value and not the found values (since this is already printed in the Specs heading).

Can you update the code to not change the value?

[hang-up]

@jennifer-shehane The side effect should have been addressed :)

[mjhenkes]

I believe this solution will break the case where we have multiple globs or a mixture of globs and specific spec files.

ie.

--spec"cypress/integration/examples/actions.spec.js,cypress/integration/examples/files.spec.js,cypress/integration/login/**/*"

indicating that you want:

• cypress/integration/examples/actions.spec.js
• cypress/integration/examples/files.spec.js
• and any specs that match cypress/integration/login/**/*

I think a better solution would involve escaping the comma and more complex parsing of the string than our current comma split.

[hang-up]

Hey @mjhenkes!
Thanks for the feedback. I've updated my test folder structure to the following:

.
├── cypress
│   ├── integration
│       └── _fixtures
│           └── fixture-should-match.spec.js        -> should match
│       └── _unit
│           └── unit-should-match.spec.js           -> should match     
│       └── login
│           └── login.spec.js                       -> should match
│       └── sub-level-1
│           └── should-not-match-level-1.spec.js                   
│         └── sub-level-2
│             └── should-not-match-level-2.spec.js                 
│       └── fixture-should-not-match.spec.js

And running a mixture of 2 specs + 1 glob pattern:

yarn cypress:run -- --spec='cypress/integration/login/login.spec.js,cypress/integration/_unit/unit-should-match.spec.js,cypress/integration/_fixtures/**/*.spec.js'

seems to pick up the right specs to run.

[mjhenkes]

I dug into the code some more, The issue will arise when there is a square or curly bracket in t regular expression. If found the whole --spec string will be treated as a single glob.

I believe the following string will fail for you (I have a different folder setup for my test repo so i can't be sure 😄 )

yarn cypress:run -- --spec='cypress/integration/login/login.spec.js,cypress/integration/_unit/unit-should-match.spec.js,cypress/integration/**/{login,fixture-should-match}.spec.js'

[hang-up]

You're right!

I've narrowed it down to a couple of possible scenarios (to make sure I'm not missing anything):

const patterns = {
  0: "cypress/integration/**/*",  // 1 glob
  1: "cypress/integration/spec1.spec.js",  // 1 spec
  2: "cypress/integration/spec1.spec.js,cypress/integration/spec2.spec.js",  // 1 spec + 1 spec
  3: "cypress/integration/spec1.spec.js,cypress/integration/glob/**/*",  //  1 spec + 1 glob
  4: "cypress/integration/spec1.spec.js,cypress/integration/{[!fix]*.spec.js,!(_unit)/**/[!fix]*.spec.js}", // 1 spec + 1 glob with lists & range
  5: "{[!fix]*.spec.js,!(_unit)/**/[!fix]*.spec.js}", // 1 glob with lists & range
  6: "cypress/{static,build/public,{src,src/increment}}/*.js", // nested ranges
  7: "cypress/integration/{[!fix]*.spec.js,! (_unit)/**/[!fix]*.spec.js},cypress/integration/spec1.spec.js,cypress/integration/{[!fix]*.specs.js,!(_unit)/**/[!fix]*.specs.js}"
}; // 2 globs with lists & ranges + 1 spec

I'll do a rewrite of the split logic to accommodate them :)

[mjhenkes]

@hang-up your spec test cases look pretty good, I don't think i have anything to add.

Dismissing my previous review

[davidmunechika]

lgtm

[cypress-bot]

Released in 9.3.0.

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