Skip to content
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

feat(SELENIUM_PROMISE_MANAGER): Don't rely on webdriver.promise fun… #82

Merged
merged 1 commit into from
Jan 21, 2017

Conversation

sjelin
Copy link
Contributor

@sjelin sjelin commented Jan 20, 2017

…ctions

While we support SELENIUM_PROMISE_MANAGER=0 already, we rely on SimpleScheduler and some other
utility functions which will be going away. This allows jasminewd to work without those utility
functions, and even allows people to pass jasminewd their own custom scheduler implementation.

This does not fix our tests, which will also break when those utility functions go away. See
#81

Closes #80

@juliemr
Copy link
Member

juliemr commented Jan 20, 2017

Please change the commit message to mention "utility function which will be going away after the control flow has been fully deprecated."

@sjelin
Copy link
Contributor Author

sjelin commented Jan 21, 2017

@juliemr done

newPromise = function(resolver) {
return new Promise(resolver);
};
}
Copy link
Member

Choose a reason for hiding this comment

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

For code readability flow, could you change the above to:

if (!scheduler) {
  scheduler = {
    execute: function(fn) {
      return Promise.resolve().then(fn);
    }
  }
}

if (typeof scheduler.promise == 'function') {
  newPromise = scheduler.promise.bind(scheduler);
} else {
  // what you have now...
}

}

// Figure out how we're getting new promises
var newPromise;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@juliemr let me know what you think of this new logic

Copy link
Member

Choose a reason for hiding this comment

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

I like not using try/catch. Works for me!

Your scheduler must emit `"idle"` when it becomes idle.


### Reset API
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@juliemr this was also added since your review

}

// Figure out how we're getting new promises
var newPromise;
Copy link
Member

Choose a reason for hiding this comment

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

I like not using try/catch. Works for me!


jasminewd will automatically look for a `reset` function and call it when specs time out. This is
useful so that if a spec executes a task that hangs, only that spec will timeout (as opposed to
tying up the scheduler and causing all future specs to timeout).
Copy link
Member

Choose a reason for hiding this comment

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

And more generally, so that if a spec times out but has lingering commands, those get cleared instead of executing in random future tests.

…ctions

While we support `SELENIUM_PROMISE_MANAGER=0` already, we rely on `SimpleScheduler` and some other
utility functions which will be going away after the control flow has been fully deprecated.  This
commit allows jasminewd to work without those utility functions, and even allows people to pass
jasminewd their own custom scheduler implementation.

This does not fix our tests, which will also break when those utility functions go away.  See
angular#81

Closes angular#80
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't rely on webdriver.promise or webdriver.promise.Scheduler
2 participants