oauth.remotepageauth doesn't close tabs successfully in Firefox

[agallant]

This test times out (doesn't get to the done call). Tweaking timeout seemed to improve it for a bit, but not reliably so. Disabling all other integration tests does allow it to pass, however.

The failing test is: https://github.com/freedomjs/freedom/blob/soycode-fixstoragetest/spec/providers/storage/storage.isolated-shared.integration.spec.js#L30-L51

And the method where it looks to hang is: https://github.com/freedomjs/freedom/blob/soycode-fixstoragetest/spec/util.js#L291-L302

Firefox 39.0.0 (Ubuntu 0.0.0) WARN '%c[Module Shared Storage]%c', 'color: red', 'color: none',
'[unbound Provider] dropping message {"to":13,"type":"message","message"
{"action":"method","type":"set","reqId":1,"text":["outerKey","outerValue"],"binary":[]}}'

That is, it doesn't even get into the first callback (the test is a sequence of 5 callbacks).

[agallant]

I think it is the oauth test somehow "leaking over", as it does run prior -

freedom/spec/providers/coreIntegration/oauth.integration.src.js

Lines 63 to 72 in 2ed5bad

	it("Fails if interactive=false and not redirected to uri", function (done) {
	var o = oauth();
	o.initiateOAuth(redirectURIs).then(function(obj) {
	var url = "http://error.com";
	var launchAuthFlowPromise = o.launchAuthFlow(url, obj, false);
	return launchAuthFlowPromise;
	}).catch(function(error) {
	done();
	});
	});

[agallant]

The issue may be that the test isn't successfully closing the tab for "error.com", and so the remaining test is running in the wrong scope. Per @dborkan the tab should be closed by: https://github.com/freedomjs/freedom/blob/master/providers/oauth/oauth.remotepageauth.js#L90

[agallant]

The current approach doesn't seem to close the tab in Chrome either, it's just Chrome continues running the tests properly and ultimately closes the whole browser after the tests complete. I've tried a few variants of window.close and similar without success.

[willscott]

Are you sure this isn't a bug associated with the longstanding issue on stability of isolated storage: #77

[agallant]

Maybe, but it is occurring very regularly post-Dan's commits and not so regularly pre, and the test does still pass if you disable other tests.

[agallant]

Also, from experimentation, tests pass if you manually monitor the Firefox browser and close the "error.com" tabs that are opened. So, either (a) the test needs to successfully close those tabs, or (b) the test needs to be refactored to not require opening new tabs. The former seems like it shouldn't be too hard, but various typical invocations of window.close are not having much luck.

[agallant]

I do think this is an actual issue with the oauth.remotepageauth provider, and that getting it to close tabs successfully will fix the test(s). I'll still work on it some, but assigning to Dan since he worked on that provider recently and has the most familiarity with the new code.

So far the approaches I've tried either don't close the tab, or close the wrong (parent/original) tab. Trying to keep a reference to the window opened doesn't seem to work as expected either.

[agallant]

For now I've disabled the test, so it's clear the build is working. Again, all tests pass when run individually (including this one), this test just needs to successfully close tabs so the remaining test(s) can pass. I do want to re-enable the test in the near future, Dan lmk thoughts/if you want to chat about how to fix. Thanks!

[agallant]

Revisiting, I think the actual issue is that this test causes Firefox to reload the page: Some of your tests did a full page reload!

Investigation suggests that Karma may be inherently against this, and a workaround (e.g. http://stackoverflow.com/questions/20029474/writing-a-test-for-a-directive-that-does-a-full-page-reload-in-karma - though this is pretty Angular specific) may be needed.