-
-
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
811216d to
62a200b
Compare
| - 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
6cb0df1 to
5a1ffe9
Compare
- Fix double negation in `Expected` with `isNot` when using `enhanceErrorBe` - Fix incorrect Received value with `isNot` when using `enhanceErrorBe` - Code refactoring simplify to better understand the code
Use Promise.all for ElementArray instead of waiting one by one Add UTs for ElementArray and Element[] Review after rebase + add awaited element case fix regression while trying to support Element[] Code review Add assertions of `executeCommandBeWithArray` and `waitUntil` All to be matchers now compliant with multiple elements + UTs fix rebase Fix ChainablePromiseElement/Array not correctly considered Even if the typing was passing using `'getElement' in received` was not considering Chainable cases Add coverage Enhance toHaveText test cases Working cases of toHaveWidth and toHaveHTML with $$() Ensure all test dans can run fast with `wait: 1` Speed-up unit tests runs Speed-up test execution Add coverage on `executeCommandWithArray` and support edge case Review error message assertions and discover a problem Have failure messages better asserted toHaveAttribute supporting $$() now Make all `toHave` matcher follow same code patterns Align code to use same code pattern and fix case of `Element[]` not working Remove `executeCommand` and simplify executeCommandWithArray + coverage Align typing with expected being an array + trim by default for arrays Support of `toHaveHeigth`, `toHaveSize` & `toHaveWidth`` fix UTs Working support of `$$()` in all element matchers Deprecate `compareTextWithArray` & remove toHaveClassContaining fix possible regression around NumberOptions wait & internal not considered Forget toHaveStyle + code review Code review better function naming Code review Code review + remove `toHaveClass` deprecated since v1, 4 versions ago Support unknown type for `toHaveElementProperty` since example existed doc: official support of `$$()` Review & add coverage + discover problem with isNot fix for other PR, waitunitl + toBeerror - to revert later? fix `.not` cases - 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 Finish matchers UT's refactor to be mocked and type safe - 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 Missed some bind Review waitUntil + coverage Better documentation Code review
5a1ffe9 to
90e551d
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