-
Notifications
You must be signed in to change notification settings - Fork 56
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
merge vyper locators natively in uiveri5 #175
base: master
Are you sure you want to change the base?
Conversation
"data-sap-ui": "*categoryList-0", | ||
"role": "option", | ||
"class": "*sapMLIB*sapMSLI", | ||
"id": "__item1-container-cart---homeView--categoryList-0" |
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.
please avoid examples with unstable ids
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.
its intented this way but will be adjusted anyway
}); | ||
|
||
it("step0v1: check name is Accessories - check with dom properties nodename and id", async function () { | ||
await browser.sleep(5000); |
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.
are all these sleeps necessary? is there something wrong with the synchronization?
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.
no not necessary again intented
*/ | ||
VyperLocator.prototype.register = function (by) { | ||
this.logger.debug('Registering custom locator'); | ||
by.ui5All = function(mMatchers) { |
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.
all to me sounds like 'element.all' and is confusing
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.
this is the way that is writen currently, it will not change yet but improvement will be considered (unfortunetaly was called like that always before we got it handover to us)
if(mParams.prevSiblingProperties || | ||
mParams.nextSiblingProperties || | ||
mParams.siblingProperties) { | ||
// ? |
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.
until we have a matcher/selector for this, log a message saying this isn't supported and will be ignored?
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.
will do, I want to write everything down and send an email with all the consolidate points
var elem = await common.locator.getDisplayedElement(ui5ControlProperties); | ||
var attribute = "tooltip"; | ||
var compareValue ="Open category Accessories"; | ||
var val = await ui5ControlConverter.getControlAggregationProperty(elem, attribute); |
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.
we have a getProperty method in uiveri5/control.js, perhaps we should just extend it with getters for aggregations and bindings?
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.
yes thats another point that in my list
"text":[{"path": "i18n>welcomeCarouselShipping"}] | ||
}} | ||
}; | ||
await ui5.common.assertion.expectAttributeToBe(ui5ControlProperties, "showHeader", "true", 0, 1, 30000); |
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.
note: we can declare the custom matchers in conf.js
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.
what do you mean custom matcher?
hi @tsaleksandrova : Thanks for the review, but its not intented on review this yet, have to adjust things first |
|
No description provided.