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: --spec now allows () in filename #29279

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

TomasTeixeira2003
Copy link

Additional details

Even though there was an existing workaround, it is easier to just allow parenthesis.

PR Tasks

@cypress-app-bot
Copy link
Collaborator

@jennifer-shehane jennifer-shehane self-requested a review April 9, 2024 17:40
cli/CHANGELOG.md Outdated Show resolved Hide resolved
@TomasTeixeira2003
Copy link
Author

TomasTeixeira2003 commented Apr 9, 2024

Is there anything I should do since the check for ''Semantic Pull Request'' is failing?

cli/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this manually and confirmed that the tests now run when specifying a spec with opening and closing parens.

Before

Screenshot 2024-04-11 at 1 53 41 PM

After

Screenshot 2024-04-11 at 1 50 03 PM

@AtofStryker AtofStryker self-requested a review April 12, 2024 13:24
Copy link
Contributor

@AtofStryker AtofStryker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if we want to escape parenthesis in all cases in cypress. the --spec arg can support glob patterns and parenthesis are a valid glob pattern, but I haven't been able to get glob patterns outside of wildcards to work correctly. @jennifer-shehane do you have more knowledge on what's expected? I mainly going off of https://docs.cypress.io/guides/guides/command-line#cypress-run-spec-lt-spec-gt and testing with the CLI and using https://globster.xyz/

cli/CHANGELOG.md Outdated Show resolved Hide resolved
@AtofStryker AtofStryker changed the title fix: --spec now allows () in filename (https://github.com/cypress-io/cypress/issues/28509) fix: --spec now allows () in filename Apr 12, 2024
jennifer-shehane and others added 2 commits April 12, 2024 17:07
Co-authored-by: Bill Glesias <bglesias@gmail.com>
@jennifer-shehane
Copy link
Member

jennifer-shehane commented Apr 12, 2024

@AtofStryker That's a good call out. This bug only happens when you have a closing paren DIRECTLY following an opening paren, so () not (a). I don't think that would be a valid usecase in any globs 🤔 , so escaping them back to back should be fine. That's what I'm thinking.

@AtofStryker
Copy link
Contributor

@jennifer-shehane is the failure in CI legit in relation to the glob changes?

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TomasTeixeira2003 There does appear to be a snapshot that has changed with globs - that is failing here. Can you take a look: https://app.circleci.com/pipelines/github/cypress-io/cypress/60955/workflows/e499d77f-ccf3-4aba-8f7f-ed00d2595ba1/jobs/2535654

Screenshot 2024-04-16 at 2 19 19 PM

@TomasTeixeira2003
Copy link
Author

I will look into it today

@jennifer-shehane
Copy link
Member

@tomasbjerre Any chance to look at this?

@tomasbjerre
Copy link
Contributor

@tomasbjerre Any chance to look at this?

@jennifer-shehane you probably tagged the wrong Tomas. But I think it looks great :)

@jennifer-shehane
Copy link
Member

Sorry @TomasTeixeira2003 Any chance to look at this?

@TomasTeixeira2003
Copy link
Author

I'm sorry, have had a lot of work lately. By tomorrow I will check it.

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.

Cypress doesn't like parenthesis () in filename
5 participants