From 24abfd8869a0e5abf3ca56d8546bfa7d12852056 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 25 Oct 2018 12:25:41 +0200 Subject: [PATCH 1/2] Wait for the promise to be fulfilled before running the next test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Jasmine 2.5 the "it" function takes an optional parameter that should be called when the async work is completed (returning a promise was introduced in Jasmine 2.7, so it is not supported yet in the tests). If the parameter is not declared then the next test is executed without waiting for the asynchronous work in the previous one to finish, which could cause that asynchronous work to finish while a different test is being run. Note that if that happens the test could still work as expected if it relied only in local variables. However, in the case of the successful revert tests, the stubs being checked are not the ones created when that test was initialized, but the ones created when the next test, the failed revert test, was initialized and the previous variables were replaced (although the model itself calls the proper stubs, as they are set through parameters in function calls). Besides all that, the checks in the failed revert test were never executed due to a different problem which will be fixed in the next commit. Signed-off-by: Daniel Calviño Sánchez --- apps/files_versions/tests/js/versionmodelSpec.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/files_versions/tests/js/versionmodelSpec.js b/apps/files_versions/tests/js/versionmodelSpec.js index 7ee239cd94a6e..d47e92e2b60de 100644 --- a/apps/files_versions/tests/js/versionmodelSpec.js +++ b/apps/files_versions/tests/js/versionmodelSpec.js @@ -58,7 +58,7 @@ describe('OCA.Versions.VersionModel', function() { model.on('revert', revertEventStub); model.on('error', errorStub); }); - it('tells the server to revert when calling the revert method', function() { + it('tells the server to revert when calling the revert method', function(done) { var promise = model.revert({ success: successStub }); @@ -76,11 +76,11 @@ describe('OCA.Versions.VersionModel', function() { expect(revertEventStub.calledOnce).toEqual(true); expect(successStub.calledOnce).toEqual(true); expect(errorStub.notCalled).toEqual(true); - }); - return promise; + done(); + }); }); - it('triggers error event when server returns a failure', function() { + it('triggers error event when server returns a failure', function(done) { var promise = model.revert({ success: successStub }); @@ -92,6 +92,8 @@ describe('OCA.Versions.VersionModel', function() { expect(revertEventStub.notCalled).toEqual(true); expect(successStub.notCalled).toEqual(true); expect(errorStub.calledOnce).toEqual(true); + + done(); }); }); }); From 661756b02ac16594a2996da2809643e10fcfc748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Thu, 25 Oct 2018 13:07:06 +0200 Subject: [PATCH 2/2] Set error expected by the DAV client for a failed move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A revert triggers a move in the DAV client, and the DAV client expects a DAV error message to be provided by the server in case of failure; if no error message is given the client ends trying to get an attribute from an undefined object and "crashes". Besides that, if the revert fails the "done" callback of the promise (the first parameter of "then") is never called, so a "fail" callback should be used instead. Signed-off-by: Daniel Calviño Sánchez --- apps/files_versions/tests/js/versionmodelSpec.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/apps/files_versions/tests/js/versionmodelSpec.js b/apps/files_versions/tests/js/versionmodelSpec.js index d47e92e2b60de..ae8801b1f5ca2 100644 --- a/apps/files_versions/tests/js/versionmodelSpec.js +++ b/apps/files_versions/tests/js/versionmodelSpec.js @@ -86,9 +86,17 @@ describe('OCA.Versions.VersionModel', function() { }); expect(fakeServer.requests.length).toEqual(1); - fakeServer.requests[0].respond(404); + var responseErrorHeaders = { + "Content-Type": "application/xml" + }; + var responseErrorBody = + '' + + ' Sabre\\DAV\\Exception\\SomeException' + + ' Some error message' + + ''; + fakeServer.requests[0].respond(404, responseErrorHeaders, responseErrorBody); - promise.then(function() { + promise.fail(function() { expect(revertEventStub.notCalled).toEqual(true); expect(successStub.notCalled).toEqual(true); expect(errorStub.calledOnce).toEqual(true);