cy.contains() improvements: normalizing whitespaces, case-sensi…

[sainthkh]

• Closes Have .contains() strip new lines of targets. #92
• Closes Introduce non case sensitive matching for cy.contains() #2785

These 2 are fixed together because the implementation for one can affect the other.

User facing changelog

• When there are multiple white spaces, it is replaced to a single white space except in <pre> tag.
• Added matchCase option to cy.contains().

Additional details

Why was this change necessary?

issue #92

Sometimes when I issue .contains() commands with text I expect to be found, it doesn't match my element as expected due to extra whitespace/new lines in my element.

HTML

<h1>hello
world</h1>

Test

cy.contains("hello world") 
// CypressError: Timed out retrying: Expected to find content: 'hello world' but never did.

issue #2785

We can currently use regex, but in some cases, we need Cypress's out-of-the-box option.

What is affected by this change?

cy.contains() can do more things like ignoring multiple spaces and case-sensitivity.

Any implementation details to explain?

A new contains implementation has been added.

How has the user experience changed?

PR Tasks

• Have tests been added/updated?
• Has a PR for user-facing changes been opened in cypress-documentation? => Cypress issue#92: cy.contains() doc. cypress-documentation#2397
• Have API changes been updated in the type definitions?~~
• Have new configuration options been added to the cypress.schema.json?

Documentation and type definition tasks will be started when this PR is ready and reviewed.

[cypress-bot]

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

• Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
• Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

• The feature/bugfix is self-documenting from within the product.
• The change provides the end user with a way to fix their problem (no dead ends).

Functionality

• The code works and performs its intended function with the correct logic.
• Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
• The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

• The code is readable (too many nested 'if's are a bad sign).
• Names used for variables, methods, etc, clearly describe their function.
• The code is easy to understood and there are relevant comments explaining.
• New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
• There are comments containing link(s) to the addressed issue (in tests and code).

Quality

• The change does not reimplement code.
• There's not a module from the ecosystem that should be used instead.
• There is no redundant or duplicate code.
• There are no irrelevant comments left in the code.
• Tests are testing the code’s intended functionality in the best way possible.

Internal

• The original issue has been tagged with a release in ZenHub.

[chrisbreiding]

Is it possible this is a breaking change? Could a user have tests that rely on the old behavior and they will fail once this is introduced? Just need to know so that we can hold off on releasing this until the next major version of Cypress.

[sainthkh]

@chrisbreiding #2785 doesn't break anything but #92 can be a breaking change. Then, should I change the target branch to v4.0-release?

[jennifer-shehane]

@sainthkh Yes, breaking changes should target v4.0-release release

[sainthkh]

Rebased and tests all passed.

[chrisbreiding]

I think the only thing left for this is to update the documentation. @sainthkh Can you open a PR targeting cypress-io/cypress-documentation#1710? Please update the cy.contains() page to detail this new behavior and update the 4.0 migration guide with an example of a test that will break with this change and how to fix it.

[sainthkh]

1. Added option type for cy.contains().
2. Wrote documentation at Cypress issue#92: cy.contains() doc. cypress-documentation#2397
3. Added a new test for leading/trailing whitespaces.

[jennifer-shehane]

why the exception for pre tags? I mean, I can kind of get the logic, but it does seem strange to make an exception for a single tag and this not be documented anywhere. I guess that's my concern. Someone opening an issue one day like, 'this is working differently here' and we have this undocumented exception.

[sainthkh]

pre tag preserves whitespaces like this:

https://codepen.io/sainthkh/pen/jOEpzmX

So, it is the exception. Because I thought it reflects how browsers work.

Do you think we need a section for this behavior in contains() documentation?

[jennifer-shehane]

Yeah...probably adding a section in the Notes of the .contains() doc so that people can find this somehow. There's are other exception notes in there.

[mariusnn]

What about elements styled with white-space: pre;?
We have cases where upgrading to Cypress 4 breaks tests where we're explicitily want to vaildate that the whitespace is present as it is rendered. Should there be a flag allowing override of this behaviour?

[sainthkh]

@mariusnn It's going on #6411.

[jennifer-shehane]

• I ran our Dashboard tests through this branch since we have about 1,000 uses of .contains() in there. I didn't see anything really strange pop up. It doesn't even seem to be breaking any of our usecases - probably because we've avoided asserting on newlines before.
• I functionally tested this on a few examples and it seems to be working as intended.

It feels a bit weird that this test case passes...

// <h1>hello world</h1>
cy.contains(/Hello World/i, { matchCase: true }) 

Also the exception of the <pre> tag seems a bit weird as undocumented behavior as mentioned in another comment.

[sainthkh]

// <h1>hello world</h1>
cy.contains(/Hello World/i, { matchCase: true }) 

In this case, how about throwing an error?:

cy.contains() option `matchCase` and regex flag conflicts. 

[sainthkh]

Now, an error is thrown for cy.contains(/Hello World/i, { matchCase: true })

[sainthkh]

Fixed how error message is thrown. But it fails a lot because of "Error: Resolution method is overspecified. Specify a callback or return a Promise; not both."

It was introduced in bcc48f2. Needs to find way to fix it.

Opened an issue at #6260.

[jennifer-shehane]

Yah, the 4.0 branch is broken atm, so trying to fix that up to get these tests passing.