Jasmine done() on setTimeout

[martinsznapka]

If you run

describe("Testing", function () {

 it("1", function () {
 expect(false).toBeTruthy();
 });

 it("2", function (done) {
 setTimeout(function () {
 expect(false).toBeTruthy();
 done();
 }, 2000);
 });

});

with protractor@3.0.0, then only first test fails.

If you run the same code with protractor@2.5.1 or with jasmine+nodejs (http://jasmine.github.io/edge/node.html) then both test fails.

[martinsznapka]

So the issue is related to protractor/node_modules/selenium-webdriver
With selenium-webdriver@2.47.0 as a Protractor dependency the above example works, but with selenium-webdriver@2.48.0 it doesn't.

There were some changes in Promise/ControlFlow:
https://github.com/SeleniumHQ/selenium/blob/master/javascript/node/selenium-webdriver/CHANGES.md#v2480

[PerBoussard]

I've been hurt by this too. Another place it behaves funny is that one can't make a spec (it()) wait for anything async in the beforeEach, which used to be possible by either declaring beforeEach to take a done callback, or by returning a promise from beforeEach. It is intermingled with so many other oddities that I just assumed my setup was completely broken (an example is buttons that element-finders mislocate and when invoking the click I get the infamous would-hit-another-element exception -- injecting javascript asking the DOM for the location of the element confirms that selenium/protractor aren't doing it right.)

[juliemr]

Confirmed - I see this error in jasminewd2 when upgrading from selenium-webdriver 2.47 to 2.48.

[juliemr]

After some investigation, I believe this is a correctly working consequence of the control flow changes in selenium-webdriver. At the root, if you schedule a task on the control flow inside a setTimeout, it will go on a new task queue, instead of being a child of the current task. In this case, this means that the task that executes expect will not be a child of the task which executes the it block. See an example here

There are two solutions here - you can use promise.delayed or flow.timeout instead of setTimeout.

We can also change the wrapping around expect so that it does not use the control flow at all if the argument is not a promise. This means that expect(true).toBe(false) will correctly fail within a timeout, but expect(myPromiseThatResolvesToTrue).toBe(false) still won't work. I'll make a PR to do that.

[juliemr]

Protractor 3.1.0 now includes this commit from JasmineWD. This is fixed, given the big caveats in my comment above.

[PerBoussard]

I find it hard to understand what changes in selenium that caused this change in behaviour, could you give me a hint? Is this somehow related to the fact that I have to wrap browser.removeMockModule() in a protractor.promise.controlFlow().execute() if I want to add and remove mock modules in the spec? Or is that an unrelated bug? Any spec with addMock();...;operate browser;...;removeMock() will see the removeMock executed prematurely since its not executed in the flow. I'm a bit confused about this -- I find it hard to believe that it's a bug nobody's been hurt by. :/

[juliemr]

@PerBoussard I wonder if maybe your issue is not related to this. Are you doing any manual setTimeout? That was the main cause of this issue. If you have another problem that you can reproduce with the latest version, please open up a new issue.

The changes in selenium were how the control flow is managed - see https://github.com/SeleniumHQ/selenium/blob/master/javascript/node/selenium-webdriver/CHANGES.md#v2480