-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
Multiremote support for TestLibrary #489
base: main
Are you sure you want to change the base?
Conversation
- initialize seperate facades for every browserInstance we have - add tests for the multiremote support
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.
scope fn questions
@@ -92,8 +92,8 @@ function initBrowser(browserInstance: WebdriverIO.Browser) { | |||
|
|||
_addWdi5Commands(browserInstance) | |||
|
|||
if (!(browserInstance as any).fe) { | |||
;(browserInstance as any).fe = WDI5FE | |||
if (!(browser as any).fe) { |
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.
hm, browserInstance
is explicitly passed in here → why is it not supposed to be used? IMO the browserInstance
helps retain scope within the fn by explicitly not referencing the global browser
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.
Then we would have to initialize the facade as often as we have defined browsers in the config. That would look like something like this:
fioriElementsFacadeOne = await one.fe.initialize{...}
fioriElementsFacadeTwo = await two.fe.initialize{...}
We can do that of course, I just don't now if this is the best way
await loadFELibraries(browserInstance) | ||
await initOPA(appConfig, browserInstance) | ||
return new WDI5FE(appConfig, browserInstance) | ||
static async initialize(appConfig) { |
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.
similar here - why remove browserInstance
as parameter and instead break scope of the otherwise "pure" function?
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 never passed the second parameter to the initialize
function. We only passed the config and just took the default for browserInstance
We could stick to that pattern, but then we have to pass the relevant browserInstance
for the multiRemote in the before
hook:
before(async () => {
FioriElementsFacade = await browser.fe.initialize({appconfig}, browserInstance)
})
I guess we just have to decide what is cleaner
valid questions. I guess we have to do a short sync to decide which way is cleaner. |
after an ad-hoc maintainer meeting (😸), we agreed to
|
Create multiple facades, depending on the amount of configured capabilities