Skip to content

Commit

Permalink
[BUGFIX release] Ensure app booted in visit helper
Browse files Browse the repository at this point in the history
Problem:

There are reported cases of the currentURL helper returning an empty
string after the release of 1.12.

Tests:

The acceptance tests are not currently checking the currentURL after
using the visit helper. This adds an assertion for the current url after
each currentRoute assertion - the currentRouteName and currentPath
helpers are functioning normally and, in these tests, the currentRoute
is a var set on transition to a new route so there is a need to
specifically check the url.

Fix:

It turns out that this isn't so much a problem with currentURL as it is
the visit helper. When the lazy router location work was completed in
8e130e5 the call to `router.location.setURL()` was moved down in the
else case after checking for readiness deferrals. The problem seems to be
a timing issue with the call to setupRouter after the application is
marked ready. The initialURL, while set, doesn't give us a reliable URL
during tests.

The call to `app.boot()` ensures the application is booted and ready,
the important part for this fix is the call to `advanceReadiness` in
boot - it is there that the router and location are set up.

After the app is booted the call to `setURL` works as expected.

Thanks to @rwjblue for working out the code needed to correct the issue.
  • Loading branch information
elskwid committed May 18, 2015
1 parent 3e804ae commit 1771aec
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
4 changes: 3 additions & 1 deletion packages/ember-testing/lib/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,15 @@ function focus(el) {

function visit(app, url) {
var router = app.__container__.lookup('router:main');
app.boot().then(function() {
router.location.setURL(url);
});

if (app._readinessDeferrals > 0) {
router['initialURL'] = url;
run(app, 'advanceReadiness');
delete router['initialURL'];
} else {
router.location.setURL(url);
run(app.__deprecatedInstance__, 'handleURL', url);
}

Expand Down
32 changes: 25 additions & 7 deletions packages/ember-testing/tests/acceptance_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import RSVP from "ember-runtime/ext/rsvp";
//ES6TODO: we need {{link-to}} and {{outlet}} to exist here
import "ember-routing"; //ES6TODO: fixme?

var App, find, click, fillIn, currentRoute, visit, originalAdapter, andThen, indexHitCount;
var App, find, click, fillIn, currentRoute, currentURL, visit, originalAdapter, andThen, indexHitCount;

QUnit.module("ember-testing Acceptance", {
setup() {
Expand Down Expand Up @@ -83,6 +83,7 @@ QUnit.module("ember-testing Acceptance", {
fillIn = window.fillIn;
visit = window.visit;
andThen = window.andThen;
currentURL = window.currentURL;

originalAdapter = Test.adapter;
},
Expand All @@ -99,12 +100,13 @@ QUnit.module("ember-testing Acceptance", {
});

QUnit.test("helpers can be chained with then", function() {
expect(5);
expect(6);

currentRoute = 'index';

visit('/posts').then(function() {
equal(currentRoute, 'posts', "Successfully visited posts route");
equal(currentURL(), '/posts', "posts URL is correct");
return click('a:contains("Comments")');
}).then(function() {
equal(currentRoute, 'comments', "visit chained with click");
Expand All @@ -125,7 +127,7 @@ QUnit.test("helpers can be chained with then", function() {
// Keep this for backwards compatibility

QUnit.test("helpers can be chained to each other", function() {
expect(5);
expect(7);

currentRoute = 'index';

Expand All @@ -134,6 +136,7 @@ QUnit.test("helpers can be chained to each other", function() {
.fillIn('.ember-text-field', "hello")
.then(function() {
equal(currentRoute, 'comments', "Successfully visited comments route");
equal(currentURL(), '/comments', "Comments URL is correct");
equal(jQuery('.ember-text-field').val(), 'hello', "Fillin successfully works");
find('.ember-text-field').one('keypress', function(e) {
equal(e.keyCode, 13, "keyevent chained with correct keyCode.");
Expand All @@ -144,11 +147,12 @@ QUnit.test("helpers can be chained to each other", function() {
.visit('/posts')
.then(function() {
equal(currentRoute, 'posts', "Thens can also be chained to helpers");
equal(currentURL(), '/posts', "URL is set correct on chained helpers");
});
});

QUnit.test("helpers don't need to be chained", function() {
expect(3);
expect(5);

currentRoute = 'index';

Expand All @@ -160,18 +164,20 @@ QUnit.test("helpers don't need to be chained", function() {

andThen(function() {
equal(currentRoute, 'comments', "Successfully visited comments route");
equal(currentURL(), '/comments', "Comments URL is correct");
equal(find('.ember-text-field').val(), 'hello', "Fillin successfully works");
});

visit('/posts');

andThen(function() {
equal(currentRoute, 'posts');
equal(currentURL(), '/posts');
});
});

QUnit.test("Nested async helpers", function() {
expect(3);
expect(5);

currentRoute = 'index';

Expand All @@ -185,18 +191,20 @@ QUnit.test("Nested async helpers", function() {

andThen(function() {
equal(currentRoute, 'comments', "Successfully visited comments route");
equal(currentURL(), '/comments', "Comments URL is correct");
equal(find('.ember-text-field').val(), 'hello', "Fillin successfully works");
});

visit('/posts');

andThen(function() {
equal(currentRoute, 'posts');
equal(currentURL(), '/posts');
});
});

QUnit.test("Multiple nested async helpers", function() {
expect(2);
expect(3);

visit('/posts');

Expand All @@ -210,11 +218,12 @@ QUnit.test("Multiple nested async helpers", function() {
andThen(function() {
equal(find('.ember-text-field').val(), 'goodbye', "Fillin successfully works");
equal(currentRoute, 'comments', "Successfully visited comments route");
equal(currentURL(), '/comments', "Comments URL is correct");
});
});

QUnit.test("Helpers nested in thens", function() {
expect(3);
expect(5);

currentRoute = 'index';

Expand All @@ -228,13 +237,15 @@ QUnit.test("Helpers nested in thens", function() {

andThen(function() {
equal(currentRoute, 'comments', "Successfully visited comments route");
equal(currentURL(), '/comments', "Comments URL is correct");
equal(find('.ember-text-field').val(), 'hello', "Fillin successfully works");
});

visit('/posts');

andThen(function() {
equal(currentRoute, 'posts');
equal(currentURL(), '/posts', "Posts URL is correct");
});
});

Expand Down Expand Up @@ -287,16 +298,21 @@ QUnit.test("Unhandled exceptions in `andThen` are logged via Ember.Test.adapter#
});

QUnit.test("should not start routing on the root URL when visiting another", function() {
expect(4);

visit('/posts');

andThen(function() {
ok(find('#comments-link'), 'found comments-link');
equal(currentRoute, 'posts', "Successfully visited posts route");
equal(currentURL(), '/posts', "Posts URL is correct");
equal(indexHitCount, 0, 'should not hit index route when visiting another route');
});
});

QUnit.test("only enters the index route once when visiting /", function() {
expect(1);

visit('/');

andThen(function() {
Expand All @@ -305,6 +321,8 @@ QUnit.test("only enters the index route once when visiting /", function() {
});

QUnit.test("test must not finish while asyncHelpers are pending", function () {
expect(2);

var async = 0;
var innerRan = false;

Expand Down

0 comments on commit 1771aec

Please sign in to comment.