From afdd9e5d3fec84119c7dfb9ae9a85dea1a40acb5 Mon Sep 17 00:00:00 2001 From: Wyatt Allen Date: Fri, 17 Jun 2016 15:46:38 -0700 Subject: [PATCH] Addresses unit-test flakiness in Safari The gr-settings-view tests partially relied on timing for evaluating the tests. If a browser acted too slowly in executing the test, the tests would occasionally fail. This change removes timing from the test and instead listens to a promise that the settings view now provides. A gr-error-manager test relied on flushing the DOM for a response content promise to be resolved. In Safari, this would frequently execute out of the expected order. This change waits for the promise itself to complete before continuing the test. Change-Id: Id93ccc44bcf3baecdba53058e42dcebccfa4c326 --- .../core/gr-error-manager/gr-error-manager_test.html | 10 +++++----- .../settings/gr-settings-view/gr-settings-view.js | 7 ++++++- .../gr-settings-view/gr-settings-view_test.html | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html index 98f79b0a86..08ce3038d8 100644 --- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html +++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html @@ -53,11 +53,11 @@ limitations under the License. test('show normal server error', function(done) { var showAlertStub = sinon.stub(element, '_showAlert'); - element.fire('server-error', {response: { - status: 500, - text: function() { return Promise.resolve('ZOMG'); }, - }}); - flush(function() { + var textSpy = sinon.spy(function() { return Promise.resolve('ZOMG'); }); + element.fire('server-error', {response: {status: 500, text: textSpy}}); + + assert.isTrue(textSpy.called); + textSpy.lastCall.returnValue.then(function() { assert.isTrue(showAlertStub.calledOnce); assert.isTrue(showAlertStub.lastCall.calledWithExactly( 'Server error: ZOMG')); diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js index 82206dadac..d9525564ce 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js @@ -76,6 +76,11 @@ type: String, value: null, }, + + /** + * For testing purposes. + */ + _loadingPromise: Object, }, observers: [ @@ -107,7 +112,7 @@ promises.push(this.$.groupList.loadData()); - Promise.all(promises).then(function() { + this._loadingPromise = Promise.all(promises).then(function() { this._loading = false; }.bind(this)); }, diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html index 8930544ff1..70b7c1c830 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html @@ -123,7 +123,7 @@ limitations under the License. element = fixture('basic'); // Allow the element to render. - element.async(done, 1); + element._loadingPromise.then(done); }); test('calls the title-change event', function() {