feat(locators): Add support for regex in cssContainingText #4532
Conversation
|
I looked at the continuous test failure, and the failures are unrelated to my changes. |
|
Not sure what to do with this: running: node scripts/interactive_tests/with_base_url.js
|
In order to get this working, I had to serialize the regex before we send it over the wire to the browser. Since there is no standard way to do this, I took guidance from a stackoverflow answer, where they call toString on the regex. Then, on the browser, you use a regex to extract out the text in between /someregex/ The hard part is to also extract out the modifiers, like i for ignore case. Anyhow, this approach seems to work, and I have tests to prove it.
joeheyming
changed the title
joeheyming
force-pushed
the
cssContainingText
branch
from
a02b669
to
cce22bb
Compare
|
Hey so... Ping.... echo... echo... echo... echoo........ |
|
Hi @joeheyming Sorry for not reacting. I'll add someone from the core team to review this PR. Can you please take a look at this PR? Tnx in advance |
| * @param {Element} using The scope of the search. | ||
| * | ||
| * @return {Array.<Element>} An array of matching elements. | ||
| */ | ||
| functions.findByCssContainingText = function(cssSelector, searchText, using) { | ||
| using = using || document; | ||
|
|
||
| if (searchText.indexOf('__REGEXP__') === 0) { | ||
| var match = searchText.split('__REGEXP__')[1].match(/\/(.*)\/(.*)?/); |
There was a problem hiding this comment.
I am not sure enough whether this match method works well here
Would you mind adding another test that using a regex with no regex flags, we can be more convinced of this change
lib/clientsidescripts.js
Outdated
| @@ -676,20 +676,26 @@ functions.findByPartialButtonText = function(searchText, using) { | |||
| * Find elements by css selector and textual content. | |||
| * | |||
| * @param {string} cssSelector The css selector to match. | |||
| * @param {string} searchText The exact text to match. | |||
| * @param {string} searchText The exact text to match. Can be a serialized regex. | |||
There was a problem hiding this comment.
Would it be better if we say: searchText The exact text to match or a serialized regex.
lib/clientsidescripts.js
Outdated
| if (elementText.indexOf(searchText) > -1) { | ||
|
|
||
| if ((searchText instanceof RegExp && searchText.test(elementText)) || | ||
| elementText.indexOf(searchText) > -1) { |
There was a problem hiding this comment.
My understanding is "elementText.indexOf(searchText) > -1" should only be applied to "searchText is not a RegExp"
For now, if elementText doesn't satisfy searchText regex, "elementText.indexOf(searchText) > -1" will still be called
There was a problem hiding this comment.
There are no side-effects to calling indexOf. The way || works is that javascript will take the first true value.
So I don't think indexOf will be called.
Here is an example I tried out:
x = () => { console.log('here'); return true; }
y = () { console.log('there'); return false; }
z = x() || y()
here
true
There was a problem hiding this comment.
the "elementText.indexOf(searchText) > -1" won't evaluate if and only if the first expression return true; however, if searchText is a RegExp and searchText.test(elementText) return false, "(searchText instanceof RegExp && searchText.test(elementText)) " will return false and the remaining indexOf will still be evaluated even though searchText is a RegExp
There was a problem hiding this comment.
Ok, I tried to address this issue. Let me know if you like the updated code.
fixed a comment, added a test.
Now, I compute the result of the client side match in a variable: elementMatches It uses a ternary to determine if we use a regex or indexOf for matching. This make the matching more explicit.
|
The last CircleCi failure was a pip install failure. Nothing to see here |
In order to get this working, I had to serialize the regex before
we send it over the wire to the browser.
Since there is no standard way to do this, I took guidance from
a stackoverflow answer, where they call toString on the regex.
Then, on the browser, you use a regex to extract out the text in between
/someregex/
The hard part is to also extract out the modifiers, like i for ignore case.
Anyhow, this approach seems to work, and I have tests to prove it.
This is related to #1360