From e65699ef7535829227563e082e667a468240a279 Mon Sep 17 00:00:00 2001 From: Benjamin Russell Date: Mon, 24 Apr 2017 13:45:33 -0700 Subject: [PATCH] Removing optional data property from Error response class (#325) * Removing optional data property from Error response class * Fixing broken unit tests --- .../Hosting/Contracts/Error.cs | 5 ---- .../Hosting/Protocol/RequestContext.cs | 7 +++-- .../Credentials/CredentialServiceTests.cs | 10 +++---- .../Formatter/TSqlFormatterServiceTests.cs | 2 +- .../LanguageServer/PeekDefinitionTests.cs | 8 +++--- .../ObjectExplorerServiceTests.cs | 12 ++++----- .../Utility/EventFlowValidator.cs | 27 +------------------ .../Utility/RequestContextMocks.cs | 6 ++--- .../Utility/TestUtils.cs | 6 ++--- 9 files changed, 26 insertions(+), 57 deletions(-) diff --git a/src/Microsoft.SqlTools.Hosting/Hosting/Contracts/Error.cs b/src/Microsoft.SqlTools.Hosting/Hosting/Contracts/Error.cs index cd1ec1a6..56f133a6 100644 --- a/src/Microsoft.SqlTools.Hosting/Hosting/Contracts/Error.cs +++ b/src/Microsoft.SqlTools.Hosting/Hosting/Contracts/Error.cs @@ -16,11 +16,6 @@ namespace Microsoft.SqlTools.Hosting.Contracts /// public int Code { get; set; } - /// - /// Optional information to return with the error - /// - public object Data { get; set; } - /// /// Error message /// diff --git a/src/Microsoft.SqlTools.Hosting/Hosting/Protocol/RequestContext.cs b/src/Microsoft.SqlTools.Hosting/Hosting/Protocol/RequestContext.cs index c0889a06..55847271 100644 --- a/src/Microsoft.SqlTools.Hosting/Hosting/Protocol/RequestContext.cs +++ b/src/Microsoft.SqlTools.Hosting/Hosting/Protocol/RequestContext.cs @@ -39,14 +39,13 @@ namespace Microsoft.SqlTools.Hosting.Protocol eventParams); } - public virtual Task SendError(string errorMessage, int errorCode = 0, object data = null) + public virtual Task SendError(string errorMessage, int errorCode = 0) { // Build the error message Error error = new Error { Message = errorMessage, - Code = errorCode, - Data = data + Code = errorCode }; return this.messageWriter.WriteMessage( Message.ResponseError( @@ -58,7 +57,7 @@ namespace Microsoft.SqlTools.Hosting.Protocol public virtual Task SendError(Exception e) { // Overload to use the parameterized error handler - return SendError(e.Message, e.HResult, e.ToString()); + return SendError(e.Message, e.HResult); } } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Credentials/CredentialServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Credentials/CredentialServiceTests.cs index df6fc9b7..d91f64b2 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Credentials/CredentialServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Credentials/CredentialServiceTests.cs @@ -76,7 +76,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials public async Task SaveCredentialThrowsIfCredentialIdMissing() { string errorResponse = null; - var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code, obj) => errorResponse = msg); + var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code) => errorResponse = msg); await service.HandleSaveCredentialRequest(new Credential(null), contextMock.Object); TestUtils.VerifyErrorSent(contextMock); @@ -87,7 +87,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials public async Task SaveCredentialThrowsIfPasswordMissing() { string errorResponse = null; - var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code, obj) => errorResponse = msg); + var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code) => errorResponse = msg); await service.HandleSaveCredentialRequest(new Credential(CredentialId), contextMock.Object); TestUtils.VerifyErrorSent(contextMock); @@ -186,7 +186,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials public async Task ReadCredentialThrowsIfCredentialIsNull() { string errorResponse = null; - var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code, obj) => errorResponse = msg); + var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code) => errorResponse = msg); // Verify throws on null, and this is sent as an error await service.HandleReadCredentialRequest(null, contextMock.Object); @@ -198,7 +198,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials public async Task ReadCredentialThrowsIfIdMissing() { string errorResponse = null; - var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code, obj) => errorResponse = msg); + var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code) => errorResponse = msg); // Verify throws with no ID await service.HandleReadCredentialRequest(new Credential(), contextMock.Object); @@ -228,7 +228,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Credentials public async Task DeleteCredentialThrowsIfIdMissing() { object errorResponse = null; - var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code, obj) => errorResponse = msg); + var contextMock = RequestContextMocks.Create(null).AddErrorHandling((msg, code) => errorResponse = msg); // Verify throws with no ID await service.HandleDeleteCredentialRequest(new Credential(), contextMock.Object); diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Formatter/TSqlFormatterServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Formatter/TSqlFormatterServiceTests.cs index 5ecd1088..135d618f 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Formatter/TSqlFormatterServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Formatter/TSqlFormatterServiceTests.cs @@ -154,7 +154,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Formatter public static void VerifyResult(Mock> contextMock, Action verify) { contextMock.Verify(c => c.SendResult(It.IsAny()), Times.Once); - contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny()), Times.Never); verify(); } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/PeekDefinitionTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/PeekDefinitionTests.cs index e13bfd6c..b10f5877 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/PeekDefinitionTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/LanguageServer/PeekDefinitionTests.cs @@ -93,9 +93,9 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer requestContext = new Mock>(); requestContext.Setup(rc => rc.SendResult(It.IsAny())) .Returns(Task.FromResult(0)); - requestContext.Setup(rc => rc.SendError(It.IsAny(), It.IsAny(), It.IsAny())).Returns(Task.FromResult(0));; - requestContext.Setup(r => r.SendEvent(It.IsAny>(), It.IsAny())).Returns(Task.FromResult(0));; - requestContext.Setup(r => r.SendEvent(It.IsAny>(), It.IsAny())).Returns(Task.FromResult(0));; + requestContext.Setup(rc => rc.SendError(It.IsAny(), It.IsAny())).Returns(Task.FromResult(0)); + requestContext.Setup(r => r.SendEvent(It.IsAny>(), It.IsAny())).Returns(Task.FromResult(0)); + requestContext.Setup(r => r.SendEvent(It.IsAny>(), It.IsAny())).Returns(Task.FromResult(0)); // setup the IBinder mock binder = new Mock(); @@ -129,7 +129,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.LanguageServer await definitionTask; // verify that send result was not called and send error was called requestContext.Verify(m => m.SendResult(It.IsAny()), Times.Never()); - requestContext.Verify(m => m.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + requestContext.Verify(m => m.SendError(It.IsAny(), It.IsAny()), Times.Once()); } /// diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/ObjectExplorerServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/ObjectExplorerServiceTests.cs index 74d32163..e144d935 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/ObjectExplorerServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/ObjectExplorerServiceTests.cs @@ -5,14 +5,14 @@ using System; using System.Threading.Tasks; -using Microsoft.SqlTools.Hosting.Protocol; +using Microsoft.SqlTools.Hosting.Protocol; using Microsoft.SqlTools.ServiceLayer.Connection; using Microsoft.SqlTools.ServiceLayer.Connection.Contracts; using Microsoft.SqlTools.ServiceLayer.ObjectExplorer; using Microsoft.SqlTools.ServiceLayer.ObjectExplorer.Contracts; using Microsoft.SqlTools.ServiceLayer.ObjectExplorer.Nodes; -using Microsoft.SqlTools.ServiceLayer.Test.Common; -using Microsoft.SqlTools.ServiceLayer.UnitTests.Utility; +using Microsoft.SqlTools.ServiceLayer.Test.Common; +using Microsoft.SqlTools.ServiceLayer.UnitTests.Utility; using Moq; using Xunit; @@ -37,7 +37,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.ObjectExplorer { object errorResponse = null; var contextMock = RequestContextMocks.Create(null) - .AddErrorHandling((errorMessage, errorCode, obj) => errorResponse = errorMessage); + .AddErrorHandling((errorMessage, errorCode) => errorResponse = errorMessage); await service.HandleCreateSessionRequest(null, contextMock.Object); VerifyErrorSent(contextMock); @@ -177,14 +177,14 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.ObjectExplorer private void VerifyResult(Mock> contextMock, Action verify, T actual) { contextMock.Verify(c => c.SendResult(It.IsAny()), Times.Once); - contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny()), Times.Never); verify(actual); } private void VerifyErrorSent(Mock> contextMock) { contextMock.Verify(c => c.SendResult(It.IsAny()), Times.Never); - contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny()), Times.Once); } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/EventFlowValidator.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/EventFlowValidator.cs index 18cbbd59..1014471a 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/EventFlowValidator.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/EventFlowValidator.cs @@ -64,37 +64,12 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility return this; } - public EventFlowValidator AddCompleteErrorValidation(Action paramValidation, - Action dataValidation) - { - // Put together a validator that checks for null and adds the provided validators - Action validator = e => - { - Assert.NotNull(e); - paramValidation(e.Message, e.Code); - - Assert.IsType(e.Data); - dataValidation((TErrorObj) e.Data); - }; - - // Add the expected error - expectedEvents.Add(new ExpectedEvent - { - EventType = EventTypes.Error, - ParamType = typeof(Error), - Validator = validator - }); - - return this; - } - public EventFlowValidator AddSimpleErrorValidation(Action paramValidation) { // Put together a validator that ensures a null data Action validator = e => { Assert.NotNull(e); - Assert.Null(e.Data); paramValidation(e.Message, e.Code); }; @@ -130,7 +105,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility .Returns(Task.FromResult(0)); // Add general handler for error event - requestContext.AddErrorHandling((msg, code, obj) => + requestContext.AddErrorHandling((msg, code) => { receivedEvents.Add(new ReceivedEvent { diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/RequestContextMocks.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/RequestContextMocks.cs index 64ae4041..b6bde5a6 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/RequestContextMocks.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/RequestContextMocks.cs @@ -48,14 +48,14 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility public static Mock> AddErrorHandling( this Mock> mock, - Action errorCallback) + Action errorCallback) { // Setup the mock for SendError - var sendErrorFlow = mock.Setup(rc => rc.SendError(It.IsAny(), It.IsAny(), It.IsAny())) + var sendErrorFlow = mock.Setup(rc => rc.SendError(It.IsAny(), It.IsAny())) .Returns(Task.FromResult(0)); if (errorCallback != null) { - sendErrorFlow.Callback(errorCallback); + sendErrorFlow.Callback(errorCallback); } return mock; diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/TestUtils.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/TestUtils.cs index 576eb2f6..ecc7a6e4 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/TestUtils.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/TestUtils.cs @@ -40,20 +40,20 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility public static void VerifyErrorSent(Mock> contextMock) { contextMock.Verify(c => c.SendResult(It.IsAny()), Times.Never); - contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny()), Times.Once); } public static void VerifyResult(Mock> contextMock, U expected, U actual) { contextMock.Verify(c => c.SendResult(It.IsAny()), Times.Once); Assert.Equal(expected, actual); - contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny()), Times.Never); } public static void VerifyResult(Mock> contextMock, Action verify, T actual) { contextMock.Verify(c => c.SendResult(It.IsAny()), Times.Once); - contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + contextMock.Verify(c => c.SendError(It.IsAny(), It.IsAny()), Times.Never); verify(actual); }