-
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
Run specs with name containing '+' #8015
Conversation
Thanks for taking the time to open a PR!
|
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.
@laistomazz Thanks! Can you add a test around this new behavior?
There is a Contributing Guide that covers how to contribute and get Cypress running locally in generally here: https://github.com/cypress-io/cypress/blob/develop/.github/CONTRIBUTING.md
Here's an example of another issue's test code below. I believe you should just be able to add a simple spec file called 5909+spec.js
, since the presence of the + would make the test fail.
To run the tests:
- within
cypress
root, runyarn
- run
yarn workspace @packages/driver cypress:open
- click on the test you're writing to run it within Cypress
Instructions for running the driver
tests can always be found here: https://github.com/cypress-io/cypress/blob/develop/packages/driver/README.md
The failing tests show that my solution doesn't work for all cases. I will look further into it. |
It was an easier fix than I thought 😅 Any suggestion on how to fix the failing test? Locally it fails on 1) e2e screenshots passes [chrome]:
AssertionError: expected 192864 not to be close to 193512 +/- 5000 |
@laistomazz just some flake, is fine now. |
Dismissing previous request for tests since tests exist now
User facing changelog
Now you can run specs with name containing '+'
Additional details
The issue is using
req.query.p
when handling the spec. This is part of an example of thereq
object:Why this happens
Express's
req.query
parser is based on Node's native query parser, querystring. It will replace+
with a space by default.Proposed solution
Quick fix: To not use the query parameter.
Proposed followup solution
Have a custom query string parsing function.
How has the user experience changed?
Now they won't see an error anymore if the spec file contains "+".