-
-
Notifications
You must be signed in to change notification settings - Fork 59
feat: Support $$() with all element's matchers
#1990
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
base: main
Are you sure you want to change the base?
Conversation
ace6980 to
2937e47
Compare
Even if the typing was passing using `'getElement' in received` was not considering Chainable cases
811216d to
62a200b
Compare
Speed-up unit tests runs Speed-up test execution
Align code to use same code pattern and fix case of `Element[]` not working
| - name: Run All Checks | ||
| run: npm run checks:all |
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.
Waiting on merge of #1991
| return aliasFn.call(this, toExist, { verb: 'be', expectation: 'existing' }, el, options) | ||
| this.verb = 'be' | ||
| this.expectation = 'existing' | ||
| this.matcherName = 'toBeExisting' |
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.
Fixed bad matcher name
$$ aka ElementArray$$ with all matchers
$$ with all matchers$$() with all matchers
$$() with all matchers$$() with all element's matchers
| return expected | ||
| } | ||
|
|
||
| export const isElementArray = (obj: unknown): obj is WebdriverIO.ElementArray => { |
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.
To review, this is probably not just ElementArray... do we need to check for foundWith = $$? Does a single element have a parent? Maybe checking for getElements would also be good...
| values: result.value | ||
| elementOrArray: isSingleElement && awaitedElements?.length === 1 ? awaitedElements[0] : awaitedElements, | ||
| success: results.length > 0 && results.every((res) => res.result === true), | ||
| valueOrArray: isSingleElement && results.length === 1 ? results[0].value : results.map(({ value }) => value), |
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.
🤔 That could unwrapped an singleton array value expecting to be an array ? To review!
| { | ||
| ignoreCase = false, | ||
| trim = false, | ||
| trim = true, |
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.
Fixed issue where single expect value vs multiple expected value behavior differently by default
| // If no options passed in + children exists | ||
| if ( | ||
| typeof options.lte !== 'number' && | ||
| typeof options.gte !== 'number' && | ||
| typeof options.eq !== 'number' | ||
| ) { | ||
| return { | ||
| result: children.length > 0, | ||
| value: children?.length | ||
| } | ||
| } | ||
|
|
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.
Replaced below by ?? { gte: 1 } + test added
const numberOptions = validateNumberOptionsArray(expectedValue ?? { gte: 1 })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.
Renaming from toHaveClass
- Ensure isNot is correct following the backport of other PR fixes - Ensure for multiple elements the not is apply on each element and fails if any case fails - Fix/consider empty elements as an error at all times
- Review all matcher UTs to call there real implementation with a this object ensuring the implementation has the right type - Use vi.mocked, to ensure we mock with the proper type
6cb0df1 to
5a1ffe9
Compare
Fixes #1507.
Matchers
toBeDisplayedand several others have TypeScript signatures that support an array element, even though they don't.Current situation
Premise
toHaveTextandtoBeElementsArrayOfSizemention$$()and therefore officially support them and must mitigate breaking changesCurrent behaviour
toHaveTextofficially supports an array for both single elements and multiple elementstoHaveHTMLdo use thecompareTextWithArray, but after testing, they do not support$$(), so their behaviour today cannot be compared totoHaveTextunder$$().$$()(await $$()) is working. Passing non-awaited$$()or$$().filter()throws an error todayError handling
$$returns only one element and we have one expected value, the error message is as below, support of this could change (to validate)$$returns only one element and an array of expectations is passed, the error message is as follows: support of this could change (to validate)Official
$$()SupportThis PR provides official support for most matchers; currently, the targets are the
toBeandtoHaveelement matchers.$$()will potentially allowexpectto work with multi-remote, this is not the target, and any side-effect of it, allowing multi-remote to work, can break at any time. Multi-remote support is tracked here but is not yet officially supported.Types Support
Behavior
The following must pass when all elements are displayed; otherwise, it fails.
toBematchers, ALL elements' value must match the boolean pass value, which is most of the timetrue, except fortoBeDisabledmatchertoHavematchers supporting an expected value, when passing an array of elements toexpect, both a single value and an array of values can be passed as the expected value, and a strict array comparison is doneStringOptions,HTMLOptions,ToBeDisplayedOptionsare, for now, kept separately and will apply to the entire array if applicable. No support per array value is plannedNumberOptionswill be allowed to be passed as multiple expected values since it is the only "option" that is really an expected value.Array Comparison Behaviour
{ trim: false }will disable that behaviour and do a strict matchtoHaveTextwill preserve the "any array value" compare behaviour to not break, but that is deprecated{ trim: false }will disable that behaviour and do a strict matchisNot
The following must pass when all elements are not displayed; otherwise, it fails.
Edge cases
.not, even if the expected is an empty array.toBeElementsArrayOfSize(0)Bugs 🐛 (fixed when checked)
$$()like inexpect($$('el')).toHaveText('text')erroring toError: Can't call "getText" on element with selector "#username", it is not a function.toBeElementsArrayOfSizeworking withElement[]but not having the typing working (fixed by Make typeElement[]work withtoBeElementsArrayOfSize#1980)toHaveElementPropertywas supporting an optional value, but using it with an existing or non-existing property always ends up in a test failure. So the typing for it was changed. Later, we could bring it back and have the same behaviour astoHaveAttribute, where it checks for the existence of the property.$$().toHaveText('C' | ['C'])that should fail when there are no elements, since we need to have elements having the text, which is not true in this case.beforeandafterhooks the wrong matcher namecontainingmatcher namedtoHaveClassContaining, deprecated for 2 years nowtoHaveClassdeprecated 4 versions agotoBeElementsArrayOfSizeawait before the waitUntil and use a refetch function, verify to move the await inside the waitUntil and remove refetch since it will be replaced by the await? Or do we need to refetch in all other matchers?Future consideration
$$(), we could support an array per element, where we can compare if that element contains those values.$$()and not$()toBeElementsArrayOfSize(0)optionslike the others; it does not convey a side-effect for the expected value, but instead is the expected value. It should be deprecated as an "options" and be an "ExpectedType" instead.wait, so deprecating it as an option would clarify that we need a separate option for those.TODO
toHaveTextsomehow now or later?Waiting on the merge of
waitUntil#1983enhanceErrorBe+ code refactoring simplification #1987