Potential fix for issue 4151: Forked driver ignores waitForAngularEnabled#4418
Potential fix for issue 4151: Forked driver ignores waitForAngularEnabled#4418slotik wants to merge 1 commit intoangular:masterfrom
Conversation
|
@cnishina Do you think you might find the time to look at this within the following week? It would be super helpful if a fix for the issue got released in the near future. |
|
@wojtek1150 Could we merge this in? Is that something I can do? |
|
when will this be available? |
|
@cnishina, @wojtek1150 @slotik @wswebcreation can we merge this? |
| */ | ||
| waitForAngularEnabled(enabled: boolean|wdpromise.Promise<boolean> = null): | ||
| wdpromise.Promise<boolean> { | ||
| waitForAngularEnabled(enabled: boolean|wdpromise.Promise<boolean> = null): boolean |
There was a problem hiding this comment.
What's the purpose of changing the return type? IMO, changing the return type will make user confused and user don't know whether the function is async or not. If you want to give a clear/accurate type inference, you could use function overloading.
| browser_.forkNewDriverInstance = | ||
| (useSameUrl: boolean, copyMockModules: boolean, copyConfigUpdates = true) => { | ||
| let newBrowser = this.createBrowser(plugins); | ||
| let newBrowser = this.createBrowser(plugins, browser_); |
There was a problem hiding this comment.
IMO, using the default config or the config of current browser for new browser, both of them make sense.
Creating browser based on parent browser just copy some init data from parent browser, I don't know why setting "waitForAngularEnabled" after creating the browser doesn't work in your case. Perhaps "waitForAngularEnabled" is async, you need wait for it? I am a little hesitate about this since I don't know whether some tests rely on the previous behavior and this change will break their tests.
6 participants
Very minimalistic candidate to fix #4151.
My hope is that it won't break other stuff.