Add new oauth integration test for interactive=false case

[dborkan]

This new test case checks that if the interactive parameter to launchAuthFlow is false, and the user is not redirected to the expected redirect url, then launchAuthFlow fails.

For this test, I had to modify jasmine.DEFAULT_TIMEOUT_INTERVAL to be 6000 ms, instead of the default 3000 ms. This is because for tab authentication in Firefox and Chrome, if 5000 ms passes with a successful redirect, we close the tab and reject the promise returned by launchAuthFlow. I tried using the jasmine.clock().tick(5000) to invoke this timeout, but that was not working. I even made a simple test case to check on the jasmine clock behavior which I think should pass but times out instead:

  it("async test", function (done) {
    jasmine.clock().install();
    setTimeout(function() {
      done();
    }, 5000);
    jasmine.clock().tick(6000);
  });

[agallant]

"Fails if interactive=false and not redirected to uri" fails for me on with the 6000ms timeout on a Linux VM (Ubuntu 14.04 1gb ram), and also looks to be timing out on Shippable. Tried increasing up to 20000ms and still failed. Not necessarily a blocking issue as it is passing on Travis, but just to note.

(The browsers launched by the test runner also load the domain parked error.com website when they hit issues, which is odd, but presumably unrelated to these changes.)

[dborkan]

I think the problem might be that Shippable is just testing this freedomjs/freedom pull request with the version of freedom-for-firefox that is currently published to npm. However for it to pass it needs the new freedom-for-firefox code in https://github.com/freedomjs/freedom-for-firefox/pull/70/files (not yet submitted). I've been testing this on my machine by building freedomjs/freedom, then symlinking to that in freedom-for-firefox with the https://github.com/freedomjs/freedom-for-firefox/pull/70/files changes and running grunt test in freedom-for-firefox

[agallant]

Gotcha - and I guess Travis is just unit tests so it passes. Thanks!

[ryscheng]

:-/ Okay well I see how this would work.

This means that the test will hang for a few seconds to pass. Are we absolutely positive jasmine's clock().tick is broken? I would find that a bit hard to believe. You'll notice the syntax is very different between jasmine versions.

https://jasmine.github.io/2.3/introduction.html#section-Jasmine_Clock https://jasmine.github.io/2.0/introduction.html#section-Mocking_the_JavaScript_Timeout_Functions https://jasmine.github.io/1.3/introduction.html#section-Mocking_the_JavaScript_Clock

[dborkan]

I followed the instructions from those docs and they aren't working either. We are using the 2.x syntax, because if I try jasmine.Clock.useMock(); I get the error jasmine.Clock.useMock is not a function.

I also tried the same type of test as in https://jasmine.github.io/2.0/introduction.html#section-Mocking_the_JavaScript_Timeout_Functions and that isn't working as advertised either - for example this test fails when I add it to oauth.integration.src.js:

  it("causes a timeout to be called synchronously", function() {
    var timerCallback = jasmine.createSpy("timerCallback");
    jasmine.clock().install();
    setTimeout(function() {
      timerCallback();
    }, 100);
    expect(timerCallback).not.toHaveBeenCalled();
    jasmine.clock().tick(101);
    expect(timerCallback).toHaveBeenCalled();
    jasmine.clock().uninstall();
  });

With error Expected spy timerCallback to have been called

Is it OK for me to just merge these pull requests for now then enter a new issue about making the jasmine clock work for the tests?

[ryscheng]

Sure, as long as we've gotten all the changes propagated to all oauth providers on firefox/chrome/freedom.js Thanks for the patience!

[dborkan]

Cool. I'm kinda confused about the order of publishing though.. Right now there is this pull request for freedom, plus https://github.com/freedomjs/freedom-for-firefox/pull/70 for firefox and https://github.com/freedomjs/freedom-for-chrome/pull/78 for freedom-for-chrome. Should I just merge all 3 of those into GitHub, then someone over at UW will publish to NPM?

[ryscheng]

Sure I can do that

On Thu, Jul 30, 2015 at 10:57 AM, dborkan notifications@github.com wrote:

Cool. I'm kinda confused about the order of publishing though.. Right now there is this pull request for freedom, plus freedomjs/freedom-for-firefox#70 https://github.com/freedomjs/freedom-for-firefox/pull/70 for firefox and freedomjs/freedom-for-chrome#78 https://github.com/freedomjs/freedom-for-chrome/pull/78 for freedom-for-chrome. Should I just merge all 3 of those into GitHub, then someone over at UW will publish to NPM?

— Reply to this email directly or view it on GitHub https://github.com/freedomjs/freedom/pull/288#issuecomment-126418280.