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

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

Merged
merged 16 commits into from
Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions cli/__snapshots__/cli_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,24 @@ exports['shows help for open --foo 1'] = `
Opens Cypress in the interactive GUI.

Options:
-b, --browser <browser-path> path to a custom browser to be added to the
-b, --browser <browser-path> path to a custom browser to be added to the
list of available browsers in Cypress
-c, --config <config> sets configuration values. separate multiple
values with a comma. overrides any value in
-c, --config <config> sets configuration values. separate multiple
values with a comma. overrides any value in
cypress.json.
-C, --config-file <config-file> path to JSON file where configuration values
are set. defaults to "cypress.json". pass
-C, --config-file <config-file> path to JSON file where configuration values
are set. defaults to "cypress.json". pass
"false" to disable.
-d, --detached [bool] runs Cypress application in detached mode
-e, --env <env> sets environment variables. separate
multiple values with a comma. overrides any
-e, --env <env> sets environment variables. separate
multiple values with a comma. overrides any
value in cypress.json or cypress.env.json
--global force Cypress into global mode as if its
--global force Cypress into global mode as if its
globally installed
-p, --port <port> runs Cypress on a specific port. overrides
-p, --port <port> runs Cypress on a specific port. overrides
any value in cypress.json.
-P, --project <project-path> path to the project
--dev runs cypress in development and bypasses
--dev runs cypress in development and bypasses
binary check
-h, --help output usage information
-------
Expand Down Expand Up @@ -198,9 +198,9 @@ exports['cli help command shows help 1'] = `
version prints Cypress version
run [options] Runs Cypress tests from the CLI without the GUI
open [options] Opens Cypress in the interactive GUI.
install [options] Installs the Cypress executable matching this package's
install [options] Installs the Cypress executable matching this package's
version
verify [options] Verifies that Cypress is installed correctly and
verify [options] Verifies that Cypress is installed correctly and
executable
cache [options] Manages the Cypress binary cache
-------
Expand Down Expand Up @@ -233,9 +233,9 @@ exports['cli help command shows help for -h 1'] = `
version prints Cypress version
run [options] Runs Cypress tests from the CLI without the GUI
open [options] Opens Cypress in the interactive GUI.
install [options] Installs the Cypress executable matching this package's
install [options] Installs the Cypress executable matching this package's
version
verify [options] Verifies that Cypress is installed correctly and
verify [options] Verifies that Cypress is installed correctly and
executable
cache [options] Manages the Cypress binary cache
-------
Expand Down Expand Up @@ -268,9 +268,9 @@ exports['cli help command shows help for --help 1'] = `
version prints Cypress version
run [options] Runs Cypress tests from the CLI without the GUI
open [options] Opens Cypress in the interactive GUI.
install [options] Installs the Cypress executable matching this package's
install [options] Installs the Cypress executable matching this package's
version
verify [options] Verifies that Cypress is installed correctly and
verify [options] Verifies that Cypress is installed correctly and
executable
cache [options] Manages the Cypress binary cache
-------
Expand Down Expand Up @@ -304,9 +304,9 @@ exports['cli unknown command shows usage and exits 1'] = `
version prints Cypress version
run [options] Runs Cypress tests from the CLI without the GUI
open [options] Opens Cypress in the interactive GUI.
install [options] Installs the Cypress executable matching this package's
install [options] Installs the Cypress executable matching this package's
version
verify [options] Verifies that Cypress is installed correctly and
verify [options] Verifies that Cypress is installed correctly and
executable
cache [options] Manages the Cypress binary cache
-------
Expand Down
6 changes: 3 additions & 3 deletions cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@
"@cypress/xvfb": "1.2.4",
"@types/sizzle": "2.3.2",
"arch": "2.1.1",
"bluebird": "3.7.1",
"bluebird": "3.7.2",
"cachedir": "2.3.0",
"chalk": "3.0.0",
"check-more-types": "2.24.0",
"commander": "4.0.1",
"commander": "4.1.0",
"common-tags": "1.8.0",
"debug": "4.1.1",
"eventemitter2": "4.1.2",
"execa": "3.3.0",
"executable": "4.1.1",
"extract-zip": "1.6.7",
"fs-extra": "8.1.0",
"getos": "3.1.1",
"getos": "3.1.4",
"is-ci": "2.0.0",
"is-installed-globally": "0.3.1",
"lazy-ass": "1.6.0",
Expand Down
18 changes: 15 additions & 3 deletions cli/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ declare namespace Cypress {
* // tries to find the given text for up to 1 second
* cy.contains('my text to find', {timeout: 1000})
*/
contains(content: string | number | RegExp, options?: Partial<Loggable & Timeoutable>): Chainable<Subject>
contains(content: string | number | RegExp, options?: Partial<Loggable & Timeoutable & CaseMatchable>): Chainable<Subject>
/**
* Get the child DOM element that contains given text.
*
Expand All @@ -612,7 +612,7 @@ declare namespace Cypress {
* // yields <ul>...</ul>
* cy.contains('ul', 'apples')
*/
contains<K extends keyof HTMLElementTagNameMap>(selector: K, text: string | number | RegExp, options?: Partial<Loggable & Timeoutable>): Chainable<JQuery<HTMLElementTagNameMap[K]>>
contains<K extends keyof HTMLElementTagNameMap>(selector: K, text: string | number | RegExp, options?: Partial<Loggable & Timeoutable & CaseMatchable>): Chainable<JQuery<HTMLElementTagNameMap[K]>>
/**
* Get the DOM element using CSS "selector" containing the text or regular expression.
*
Expand All @@ -621,7 +621,7 @@ declare namespace Cypress {
* // yields <... class="foo">... apples ...</...>
* cy.contains('.foo', 'apples')
*/
contains<E extends Node = HTMLElement>(selector: string, text: string | number | RegExp, options?: Partial<Loggable & Timeoutable>): Chainable<JQuery<E>>
contains<E extends Node = HTMLElement>(selector: string, text: string | number | RegExp, options?: Partial<Loggable & Timeoutable & CaseMatchable>): Chainable<JQuery<E>>

/**
* Double-click a DOM element.
Expand Down Expand Up @@ -1995,6 +1995,18 @@ declare namespace Cypress {
timeout: number
}

/**
* Options that check case sensitivity
*/
interface CaseMatchable {
/**
* Check case sensitivity
*
* @default true
*/
matchCase: boolean
}

/**
* Options that control how long the Test Runner waits for an XHR request and response to succeed
*/
Expand Down
40 changes: 38 additions & 2 deletions packages/driver/src/cy/commands/querying.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const restoreContains = () => {
return $expr.contains = $contains
}

const whitespaces = /\s+/g

module.exports = (Commands, Cypress, cy) => {
// restore initially when a run starts
restoreContains()
Expand Down Expand Up @@ -407,7 +409,11 @@ module.exports = (Commands, Cypress, cy) => {
filter = ''
}

_.defaults(options, { log: true })
if (options.matchCase === true && _.isRegExp(text) && text.flags.includes('i')) {
$utils.throwErrByPath('contains.regex_conflict')
}

_.defaults(options, { log: true, matchCase: true })

if (!(_.isString(text) || _.isFinite(text) || _.isRegExp(text))) {
$utils.throwErrByPath('contains.invalid_argument')
Expand Down Expand Up @@ -477,10 +483,40 @@ module.exports = (Commands, Cypress, cy) => {
options._log.set({ $el })
}

// When multiple space characters are considered as a single whitespace in all tags except <pre>.
const normalizeWhitespaces = (elem) => {
let testText = elem.textContent || elem.innerText || $.text(elem)

if (elem.tagName === 'PRE') {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@sainthkh sainthkh Jan 15, 2020

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mariusnn It's going on #6411.

return testText
}

return testText.replace(whitespaces, ' ')
}

if (_.isRegExp(text)) {
if (options.matchCase === false && !text.flags.includes('i')) {
text = new RegExp(text.source, text.flags + 'i') // eslint-disable-line prefer-template
}

// taken from jquery's normal contains method
$expr.contains = (elem) => {
return text.test(elem.textContent || elem.innerText || $.text(elem))
let testText = normalizeWhitespaces(elem)

return text.test(testText)
}
}

if (_.isString(text)) {
$expr.contains = (elem) => {
let testText = normalizeWhitespaces(elem)

if (!options.matchCase) {
testText = testText.toLowerCase()
text = text.toLowerCase()
}

return testText.includes(text)
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/driver/src/cypress/error_messages.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ module.exports = {
empty_string: "#{cmd('contains')} cannot be passed an empty string."
invalid_argument: "#{cmd('contains')} can only accept a string, number or regular expression."
length_option: "#{cmd('contains')} cannot be passed a length option because it will only ever return 1 element."
regex_conflict: "You passed a regular expression with the case-insensitive (i) flag and { matchCase: true } to #{cmd('contains')}. Those options conflict with each other, so please choose one or the other."

cookies:
backend_error: """
Expand Down
116 changes: 112 additions & 4 deletions packages/driver/test/cypress/integration/commands/querying_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1817,21 +1817,129 @@ describe('src/cy/commands/querying', () => {
})
})

// NOTE: not sure why this is skipped... last edit was 3 years ago...
// @bkucera maybe take a look at this
describe.skip('handles whitespace', () => {
describe('handles whitespace', () => {
it('finds el with new lines', () => {
const btn = $(`\
<button id="whitespace">
<button id="whitespace1">
White
space
</button>\
`).appendTo(cy.$$('body'))

cy.get('#whitespace1').contains('White space')
cy.contains('White space').then(($btn) => {
expect($btn.get(0)).to.eq(btn.get(0))
})
})

it('finds el with new lines + spaces', () => {
const btn = $(`\
<button id="whitespace2">
White
space
</button>\
`).appendTo(cy.$$('body'))

cy.get('#whitespace2').contains('White space')
cy.contains('White space').then(($btn) => {
expect($btn.get(0)).to.eq(btn.get(0))
})
})

it('finds el with multiple spaces', () => {
const btn = $(`\
<button id="whitespace3">
White space
</button>\
`).appendTo(cy.$$('body'))

cy.get('#whitespace3').contains('White space')
cy.contains('White space').then(($btn) => {
expect($btn.get(0)).to.eq(btn.get(0))
})
})

it('finds el with regex', () => {
const btn = $(`\
<button id="whitespace4">
White space
</button>\
`).appendTo(cy.$$('body'))

cy.get('#whitespace4').contains('White space')
cy.contains(/White space/).then(($btn) => {
expect($btn.get(0)).to.eq(btn.get(0))
})
})

it('does not normalize text in pre tag', () => {
$(`\
<pre id="whitespace5">
White
space
</pre>\
`).appendTo(cy.$$('body'))

cy.contains('White space').should('not.match', 'pre')
cy.get('#whitespace5').contains('White\nspace')
})

it('finds el with leading/trailing spaces', () => {
const btn = $(`<button id="whitespace6"> White space </button>`).appendTo(cy.$$('body'))

cy.get('#whitespace6').contains('White space')
cy.contains('White space').then(($btn) => {
expect($btn.get(0)).to.eq(btn.get(0))
})
})
})

describe('case sensitivity', () => {
beforeEach(() => {
$('<button id="test-button">Test</button>').appendTo(cy.$$('body'))
})

it('is case sensitive when matchCase is undefined', () => {
cy.get('#test-button').contains('Test')
})

it('is case sensitive when matchCase is true', () => {
cy.get('#test-button').contains('Test', {
matchCase: true,
})
})

it('is case insensitive when matchCase is false', () => {
cy.get('#test-button').contains('test', {
matchCase: false,
})

cy.get('#test-button').contains(/Test/, {
matchCase: false,
})
})

it('does not crash when matchCase: false is used with regex flag, i', () => {
cy.get('#test-button').contains(/Test/i, {
matchCase: false,
})
})

it('throws when content has "i" flag while matchCase: true', (done) => {
cy.on('fail', (err) => {
expect(err.message).to.eq('You passed a regular expression with the case-insensitive (i) flag and { matchCase: true } to cy.contains(). Those options conflict with each other, so please choose one or the other.')

done()
})

cy.get('#test-button').contains(/Test/i, {
matchCase: true,
})
})

it('passes when "i" flag is used with undefined option', () => {
cy.get('#test-button').contains(/Test/i)
})
})

describe('subject contains text nodes', () => {
Expand Down