From 6e32f4e0116645204288f0aa937141d197aa2822 Mon Sep 17 00:00:00 2001 From: Krati Ahuja Date: Sun, 1 Oct 2017 15:33:08 -0700 Subject: [PATCH] [BUGFIX lts] Add failing test for error template --- package.json | 1 + .../integration/application/engine-test.js | 8 +- packages/ember-routing/lib/system/router.js | 4 + packages/ember/tests/routing/basic_test.js | 2 +- .../ember/tests/routing/substates_test.js | 123 +++++++++--------- tests/node/visit-test.js | 62 ++++++--- 6 files changed, 113 insertions(+), 87 deletions(-) diff --git a/package.json b/package.json index c0952c0f79e..251ab10e9e9 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "pretest": "ember build", "test": "node bin/run-tests.js", "test:blueprints": "node node-tests/nodetest-runner.js", + "test:node": "node bin/run-node-tests.js", "test:sauce": "node bin/run-sauce-tests.js", "test:testem": "testem -f testem.dist.json", "link:glimmer": "node bin/yarn-link-glimmer.js" diff --git a/packages/ember-glimmer/tests/integration/application/engine-test.js b/packages/ember-glimmer/tests/integration/application/engine-test.js index b3a6dc757bc..c09248b07a5 100644 --- a/packages/ember-glimmer/tests/integration/application/engine-test.js +++ b/packages/ember-glimmer/tests/integration/application/engine-test.js @@ -426,7 +426,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest { return this.visit('/').then(() => { this.assertText('Application'); return this.transitionTo('blog.post'); - }).catch(() => { + }).then(() => { return errorEntered.promise; }).then(() => { this.assertText('ApplicationError! Oh, noes!'); @@ -456,7 +456,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest { return this.visit('/').then(() => { this.assertText('Application'); return this.transitionTo('blog.post'); - }).catch(() => { + }).then(() => { return errorEntered.promise; }).then(() => { this.assertText('ApplicationEngineError! Oh, noes!'); @@ -486,7 +486,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest { return this.visit('/').then(() => { this.assertText('Application'); return this.transitionTo('blog.post'); - }).catch(() => { + }).then(() => { return errorEntered.promise; }).then(() => { this.assertText('ApplicationEngineError! Oh, noes!'); @@ -516,7 +516,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest { return this.visit('/').then(() => { this.assertText('Application'); return this.transitionTo('blog.post.comments'); - }).catch(() => { + }).then(() => { return errorEntered.promise; }).then(() => { this.assertText('ApplicationEngineError! Oh, noes!'); diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js index 4d512dac613..d29865fc461 100644 --- a/packages/ember-routing/lib/system/router.js +++ b/packages/ember-routing/lib/system/router.js @@ -1126,6 +1126,8 @@ let defaultActionHandlers = { // Check for the existence of an 'error' route. let errorRouteName = findRouteStateName(route, 'error'); if (errorRouteName) { + let errorId = guidFor(error); + router._markErrorAsHandled(errorId); router.intermediateTransitionTo(errorRouteName, error); return false; } @@ -1134,6 +1136,8 @@ let defaultActionHandlers = { // Check for an 'error' substate route let errorSubstateName = findRouteSubstateName(route, 'error'); if (errorSubstateName) { + var errorId = guidFor(error); + router._markErrorAsHandled(errorId); router.intermediateTransitionTo(errorSubstateName, error); return false; } diff --git a/packages/ember/tests/routing/basic_test.js b/packages/ember/tests/routing/basic_test.js index f342213594d..e96f6a9fa35 100644 --- a/packages/ember/tests/routing/basic_test.js +++ b/packages/ember/tests/routing/basic_test.js @@ -2714,7 +2714,7 @@ QUnit.test('Errors in transition show error template if available', function() { } }); - throws(() => bootApplication(), /More context objects were passed/); + bootApplication(); equal(jQuery('#error').length, 1, 'Error template was rendered.'); }); diff --git a/packages/ember/tests/routing/substates_test.js b/packages/ember/tests/routing/substates_test.js index 94b312f03b4..88483c7a039 100644 --- a/packages/ember/tests/routing/substates_test.js +++ b/packages/ember/tests/routing/substates_test.js @@ -352,15 +352,15 @@ moduleFor('Loading/Error Substates', class extends ApplicationTestCase { })); return this.visit('/').then(() => { - assert.throws(() => { - this.visit('/foo/bar'); - }, (err) => err.msg === "did it broke?", 'it broke'); - let text = this.$('#app').text(); - assert.equal( - text, - 'FOOBAR ERROR: did it broke?', - `foo.bar_error was entered (as opposed to something like foo/foo/bar_error)` - ); + return this.visit('/foo/bar').then(() => { + + let text = this.$('#app').text(); + assert.equal( + text, + 'FOOBAR ERROR: did it broke?', + `foo.bar_error was entered (as opposed to something like foo/foo/bar_error)` + ); + }); }); } ['@test Prioritized loading substate entry works with auto-generated index routes'](assert) { @@ -424,16 +424,16 @@ moduleFor('Loading/Error Substates', class extends ApplicationTestCase { })); return this.visit('/').then(() => { - assert.throws(() => { - this.visit('/foo'); - }, (err) => err.msg === 'did it broke?', 'it broke'); - let text = this.$('#app').text(); - - assert.equal( - text, - 'FOO ERROR: did it broke?', - 'foo.index_error was entered' - ); + + return this.visit('/foo').then(() => { + let text = this.$('#app').text(); + + assert.equal( + text, + 'FOO ERROR: did it broke?', + 'foo.index_error was entered' + ); + }); }); } }); @@ -461,28 +461,26 @@ moduleFor('Loading/Error Substates - globals mode app', class extends AutobootAp ['@test Rejected promises returned from ApplicationRoute transition into top-level application_error'](assert) { let reject = true; - assert.throws(() => { - this.runTask(() => { - this.createApplication(); - this.addTemplate('index', '
INDEX
'); - this.add('route:application', Route.extend({ - init() { - this._super(...arguments); - }, - model() { - if (reject) { - return RSVP.reject({ msg: 'BAD NEWS BEARS' }); - } else { - return {}; - } + this.runTask(() => { + this.createApplication(); + this.addTemplate('index', '
INDEX
'); + this.add('route:application', Route.extend({ + init() { + this._super(...arguments); + }, + model() { + if (reject) { + return RSVP.reject({ msg: 'BAD NEWS BEARS' }); + } else { + return {}; } - })); + } + })); - this.addTemplate('application_error', ` -

TOPLEVEL ERROR: {{model.msg}}

- `); - }) - }, (err) => err.msg === 'BAD NEWS BEARS', 'it went poorly'); + this.addTemplate('application_error', ` +

TOPLEVEL ERROR: {{model.msg}}

+ `); + }); let text = this.$('#toplevel-error').text(); assert.equal( @@ -645,16 +643,15 @@ moduleFor('Loading/Error Substates - nested routes', class extends ApplicationTe } })); - assert.throws(() => { - this.visit('/grandma/mom/sally'); - }, (err) => err.msg === "did it broke?", 'it broke.'); - - step(3, 'App finished loading'); - - let text = this.$('#app').text(); - - assert.equal(text, 'GRANDMA ERROR: did it broke?', 'error bubbles'); - assert.equal(this.currentPath, 'grandma.error', 'Initial route fully loaded'); + + return this.visit('/grandma/mom/sally').then(() => { + step(3, 'App finished loading'); + + let text = this.$('#app').text(); + + assert.equal(text, 'GRANDMA ERROR: did it broke?', 'error bubbles'); + assert.equal(this.currentPath, 'grandma.error', 'Initial route fully loaded'); + }); } [`@test Non-bubbled errors that re-throw aren't swallowed`](assert) { @@ -783,21 +780,19 @@ moduleFor('Loading/Error Substates - nested routes', class extends ApplicationTe } })); - assert.throws(() => { - this.visit('/grandma/mom/sally'); - }, (err) => err.msg === 'did it broke?', 'it broke'); - - step(3, 'Application finished booting'); - - assert.equal( - this.$('#app').text(), - 'GRANDMA MOM ERROR: did it broke?', - 'the more specifically named mome error substate was entered over the other error route' - ); - - assert.equal(this.currentPath, 'grandma.mom_error', - 'Initial route fully loaded' - ); + return this.visit('/grandma/mom/sally').then(() => { + step(3, 'Application finished booting'); + + assert.equal( + this.$('#app').text(), + 'GRANDMA MOM ERROR: did it broke?', + 'the more specifically named mome error substate was entered over the other error route' + ); + + assert.equal(this.currentPath, 'grandma.mom_error', + 'Initial route fully loaded' + ); + }); } ['@test Slow promises waterfall on startup'](assert) { diff --git a/tests/node/visit-test.js b/tests/node/visit-test.js index 28a7782416e..18c6235015c 100644 --- a/tests/node/visit-test.js +++ b/tests/node/visit-test.js @@ -170,24 +170,50 @@ QUnit.test('FastBoot: route error', function(assert) { var App = this.createApplication(); return RSVP.all([ - fastbootVisit(App, '/a').then( - function(instance) { - assert.ok(false, 'It should not render'); - instance.destroy(); - }, - function(error) { - assert.equal(error.message, 'Error from A'); - } - ), - fastbootVisit(App, '/b').then( - function(instance) { - assert.ok(false, 'It should not render'); - instance.destroy(); - }, - function(error) { - assert.equal(error.message, 'Error from B'); - } - ) + fastbootVisit(App, '/a') + .then( + function(instance) { + assert.ok(false, 'It should not render'); + instance.destroy(); + }, + function(error) { + assert.equal(error.message, 'Error from A'); + } + ), + fastbootVisit(App, '/b').then( + function(instance) { + assert.ok(false, 'It should not render'); + instance.destroy(); + }, + function(error) { + assert.equal(error.message, 'Error from B'); + } + ) + ]); +}); + +QUnit.test('FastBoot: route error template', function(assert) { + this.routes(function() { + this.route('a'); + }); + + this.template('error', '

Error template rendered!

'); + this.template('a', '

Hello from A

'); + + this.route('a', { + model: function() { + throw new Error('Error from A'); + } + }); + + var App = this.createApplication(); + + return RSVP.all([ + fastbootVisit(App, '/a') + .then( + assertFastbootResult(assert, { url: '/a', body: '

Error template rendered!

' }), + handleError(assert) + ), ]); });