Skip to content
This repository was archived by the owner on Jul 29, 2024. It is now read-only.

Potential fix for issue 4151: Forked driver ignores waitForAngularEnabled #4418

Open
slotik wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Potential fix for issue 4151: Forked driver ignores waitForAngularEnabled #4418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,8 +417,8 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
* Call waitForAngularEnabled() without passing a value to read the current
* state without changing it.
*/
waitForAngularEnabled(enabled: boolean|wdpromise.Promise<boolean> = null):
wdpromise.Promise<boolean> {
waitForAngularEnabled(enabled: boolean|wdpromise.Promise<boolean> = null): boolean
Copy link
Copy Markdown
Contributor

@qiyigg qiyigg May 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

|wdpromise.Promise<boolean> {
if (enabled != null) {
const ret = this.driver.controlFlow().execute(() => {
return wdpromise.when(enabled).then((enabled: boolean) => {
Expand All @@ -433,7 +433,7 @@ export class ProtractorBrowser extends AbstractExtendedWebDriver {
this.internalIgnoreSynchronization = !enabled;
return ret;
}
return wdpromise.when(!this.ignoreSynchronization);
return !this.ignoreSynchronization;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/runner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export class Runner extends EventEmitter {

browser_.forkNewDriverInstance =
(useSameUrl: boolean, copyMockModules: boolean, copyConfigUpdates = true) => {
let newBrowser = this.createBrowser(plugins);
let newBrowser = this.createBrowser(plugins, browser_);
Copy link
Copy Markdown
Contributor

@qiyigg qiyigg May 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (copyMockModules) {
newBrowser.mockModules_ = browser_.mockModules_;
}
Expand Down