diff --git a/src/ConnectedMode.UnitTests/Binding/Suggestion/SuggestSharedBindingGoldBarTests.cs b/src/ConnectedMode.UnitTests/Binding/Suggestion/SuggestSharedBindingGoldBarTests.cs index 4ba7d0bba4..bda5d92cea 100644 --- a/src/ConnectedMode.UnitTests/Binding/Suggestion/SuggestSharedBindingGoldBarTests.cs +++ b/src/ConnectedMode.UnitTests/Binding/Suggestion/SuggestSharedBindingGoldBarTests.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Linq; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Binding.Suggestion; using SonarLint.VisualStudio.Core; @@ -153,7 +152,7 @@ public void Close_RemovesGoldBar() testSubject.Close(); - notificationServiceMock.Verify(x => x.RemoveNotification(), Times.Once); + notificationServiceMock.Verify(x => x.CloseNotification(), Times.Once); } private SuggestSharedBindingGoldBar CreateTestSubject(INotificationService notificationServiceMock, diff --git a/src/ConnectedMode.UnitTests/Migration/MigrationPromptTests.cs b/src/ConnectedMode.UnitTests/Migration/MigrationPromptTests.cs index 14bd80312e..14110997f6 100644 --- a/src/ConnectedMode.UnitTests/Migration/MigrationPromptTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/MigrationPromptTests.cs @@ -18,10 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; -using System.Linq; -using Microsoft.VisualStudio.Shell; -using Microsoft.VisualStudio.Threading; using SonarLint.VisualStudio.ConnectedMode.Migration; using SonarLint.VisualStudio.ConnectedMode.Migration.Wizard; using SonarLint.VisualStudio.Core; @@ -141,7 +137,7 @@ public void FinishMigrationRaised_ClearIsCalled() _ = CreateTestSubject(notificationService: notificationService.Object, migrationWizardController: migrationWizardController.Object); migrationWizardController.Raise(x => x.MigrationWizardFinished += null, EventArgs.Empty); - notificationService.Verify(x => x.RemoveNotification(), Times.Once); + notificationService.Verify(x => x.CloseNotification(), Times.Once); } [TestMethod] @@ -152,7 +148,7 @@ public void Clear_RemoveNotificationIsCalled() var testSubject = CreateTestSubject(notificationService: notificationService.Object); testSubject.Clear(); - notificationService.Verify(x => x.RemoveNotification(), Times.Once); + notificationService.Verify(x => x.CloseNotification(), Times.Once); } [TestMethod] @@ -167,7 +163,7 @@ public void Dispose_UnsubscribesFromEvents() migrationWizardController.Raise(x => x.MigrationWizardFinished += null, EventArgs.Empty); - notificationService.Verify(x => x.RemoveNotification(), Times.Never); + notificationService.Verify(x => x.CloseNotification(), Times.Never); } private ISolutionInfoProvider CreateSolutionInfoProvider(string pathToSolution = "") diff --git a/src/ConnectedMode/Binding/Suggestion/SuggestSharedBindingGoldBar.cs b/src/ConnectedMode/Binding/Suggestion/SuggestSharedBindingGoldBar.cs index d8ad98e45e..ea16a4f59d 100644 --- a/src/ConnectedMode/Binding/Suggestion/SuggestSharedBindingGoldBar.cs +++ b/src/ConnectedMode/Binding/Suggestion/SuggestSharedBindingGoldBar.cs @@ -18,7 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; using System.ComponentModel.Composition; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Core.Notifications; @@ -73,7 +72,7 @@ public void Show(ServerType serverType, Action onConnectHandler) public void Close() { - notificationService.RemoveNotification(); + notificationService.CloseNotification(); } private void OnLearnMore() diff --git a/src/ConnectedMode/Migration/IMigrationPrompt.cs b/src/ConnectedMode/Migration/IMigrationPrompt.cs index 132a46ef30..7c86e39f7a 100644 --- a/src/ConnectedMode/Migration/IMigrationPrompt.cs +++ b/src/ConnectedMode/Migration/IMigrationPrompt.cs @@ -18,13 +18,11 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; using System.ComponentModel.Composition; using SonarLint.VisualStudio.ConnectedMode.Migration.Wizard; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Core.Binding; using SonarLint.VisualStudio.Core.Notifications; - using Task = System.Threading.Tasks.Task; namespace SonarLint.VisualStudio.ConnectedMode.Migration @@ -101,7 +99,7 @@ public async Task ShowAsync(BoundSonarQubeProject oldBinding, bool hasNewBinding public void Clear() { - notificationService.RemoveNotification(); + notificationService.CloseNotification(); } private void OnMigrate() diff --git a/src/Core.UnitTests/Notifications/NotificationServiceTests.cs b/src/Core.UnitTests/Notifications/NotificationServiceTests.cs index 9b2a700a02..dacd1e15df 100644 --- a/src/Core.UnitTests/Notifications/NotificationServiceTests.cs +++ b/src/Core.UnitTests/Notifications/NotificationServiceTests.cs @@ -59,7 +59,7 @@ public void ShowNotification_ShouldAddInfoBarOnUiThread() { var notification = CreateNotification(); var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var threadHandling = new Mock(); Action runOnUiAction = null; @@ -94,7 +94,7 @@ public void ShowNotification_InfoBarCreatedCorrectly() }); var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var testSubject = CreateTestSubject(infoBarManager.Object); testSubject.ShowNotification(notification); @@ -111,7 +111,7 @@ public void ShowNotification_NotificationIsDisabled_NotificationNotShown() var notification = CreateNotification(id: "some id"); var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var disabledNotificationsStorage = new Mock(); disabledNotificationsStorage.Setup(x => x.IsNotificationDisabled("some id")).Returns(true); @@ -137,8 +137,8 @@ public void ShowNotification_NotificationIsDisabled_PreviousNotificationNotRemov var infoBar2 = CreateInfoBar(); var infoBarManager = new Mock(); - SetupInfoBarManager(infoBarManager, notification1, infoBar1.Object); - SetupInfoBarManager(infoBarManager, notification2, infoBar2.Object); + SetupInfoBarManager(infoBarManager, notification1, infoBar1); + SetupInfoBarManager(infoBarManager, notification2, infoBar2); var disabledNotificationsStorage = new Mock(); disabledNotificationsStorage.Setup(x => x.IsNotificationDisabled("some id1")).Returns(false); @@ -178,9 +178,9 @@ public void ShowNotification_OncePerSessionEnaled_NotificationWithSameIdAlreadyS var infoBar3 = CreateInfoBar(); var infoBarManager = new Mock(); - SetupInfoBarManager(infoBarManager, notification1, infoBar1.Object); - SetupInfoBarManager(infoBarManager, notification2, infoBar2.Object); - SetupInfoBarManager(infoBarManager, notification3, infoBar3.Object); + SetupInfoBarManager(infoBarManager, notification1, infoBar1); + SetupInfoBarManager(infoBarManager, notification2, infoBar2); + SetupInfoBarManager(infoBarManager, notification3, infoBar3); var testSubject = CreateTestSubject(infoBarManager.Object); @@ -215,9 +215,9 @@ public void ShowNotification_OncePerSessionDisabled_NotificationWithSameIdAlread var infoBar3 = CreateInfoBar(); var infoBarManager = new Mock(); - SetupInfoBarManager(infoBarManager, notification1, infoBar1.Object); - SetupInfoBarManager(infoBarManager, notification2, infoBar2.Object); - SetupInfoBarManager(infoBarManager, notification3, infoBar3.Object); + SetupInfoBarManager(infoBarManager, notification1, infoBar1); + SetupInfoBarManager(infoBarManager, notification2, infoBar2); + SetupInfoBarManager(infoBarManager, notification3, infoBar3); var testSubject = CreateTestSubject(infoBarManager.Object); @@ -251,7 +251,7 @@ public void ShowNotification_UnknownInfoBarButtonClicked_NoException() var infoBar = new Mock(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var testSubject = CreateTestSubject(infoBarManager.Object); testSubject.ShowNotification(notification); @@ -277,7 +277,7 @@ public void ShowNotification_InfoBarButtonClicked_ActionInvokedWithTheNotificati var infoBar = new Mock(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var testSubject = CreateTestSubject(infoBarManager.Object); testSubject.ShowNotification(notification); @@ -305,7 +305,7 @@ public void ShowNotification_InfoBarButtonClicked_CorrectActionInvoked() var infoBar = new Mock(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var testSubject = CreateTestSubject(infoBarManager.Object); testSubject.ShowNotification(notification); @@ -333,8 +333,8 @@ public void ShowNotification_HasPreviousNotification_PreviousNotificationRemoved var infoBar2 = CreateInfoBar(); var infoBarManager = new Mock(); - SetupInfoBarManager(infoBarManager, notification1, infoBar1.Object); - SetupInfoBarManager(infoBarManager, notification2, infoBar2.Object); + SetupInfoBarManager(infoBarManager, notification1, infoBar1); + SetupInfoBarManager(infoBarManager, notification2, infoBar2); var testSubject = CreateTestSubject(infoBarManager.Object); @@ -350,26 +350,6 @@ public void ShowNotification_HasPreviousNotification_PreviousNotificationRemoved VerifySubscribedToInfoBarEvents(infoBar2); } - [TestMethod] - public void ShowNotification_UserClosedTheNotification_NotificationRemoved() - { - var notification = CreateNotification(); - var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); - var testSubject = CreateTestSubject(infoBarManager.Object); - - testSubject.ShowNotification(notification); - - VerifyInfoBarCreatedCorrectly(infoBarManager, notification); - VerifySubscribedToInfoBarEvents(infoBar); - - infoBar.Raise(x => x.Closed += null, EventArgs.Empty); - - VerifyInfoBarRemoved(infoBarManager, infoBar); - - infoBarManager.VerifyNoOtherCalls(); - } - [TestMethod] public void Dispose_NoExistingNotification_NoException() { @@ -442,7 +422,7 @@ public void ShowNotification_InfoBarButtonClicked_NonCriticalException_Exception var infoBar = new Mock(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); var logger = new TestLogger(); var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger); @@ -454,56 +434,13 @@ public void ShowNotification_InfoBarButtonClicked_NonCriticalException_Exception logger.AssertPartialOutputStringExists("this is a test"); } - [TestMethod] - public void ShowNotification_InfoBarButtonClicked_CriticalException_ExceptionNotCaught() - { - var notification = CreateNotification(actions: - new NotificationAction("action", _ => throw new StackOverflowException("this is a test"), false) - ); - - var infoBar = new Mock(); - - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); - var logger = new TestLogger(); - - var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger); - testSubject.ShowNotification(notification); - - Action act = () => infoBar.Raise(x => x.ButtonClick += null, new InfoBarButtonClickedEventArgs("action")); - - act.Should().ThrowExactly().WithMessage("this is a test"); - logger.AssertNoOutputMessages(); - } - - [TestMethod] - public void ShowNotification_UserClosedTheNotification_NonCriticalException_ExceptionCaught() - { - var notification = CreateNotification(); - var infoBar = CreateInfoBar(); - - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); - infoBarManager - .Setup(x => x.CloseInfoBar(infoBar.Object)) - .Throws(new NotImplementedException("this is a test")); - - var logger = new TestLogger(); - - var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger); - testSubject.ShowNotification(notification); - - Action act = () => infoBar.Raise(x => x.Closed += null, EventArgs.Empty); - act.Should().NotThrow(); - - logger.AssertPartialOutputStringExists("this is a test"); - } - [TestMethod] public void ShowNotification_UserClosedTheNotification_CriticalException_ExceptionNotCaught() { var notification = CreateNotification(); var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); infoBarManager .Setup(x => x.CloseInfoBar(infoBar.Object)) .Throws(new StackOverflowException("this is a test")); @@ -512,26 +449,26 @@ public void ShowNotification_UserClosedTheNotification_CriticalException_Excepti var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger); testSubject.ShowNotification(notification); - - Action act = () => infoBar.Raise(x => x.Closed += null, EventArgs.Empty); + + var act = () => testSubject.CloseNotification(); act.Should().ThrowExactly().WithMessage("this is a test"); logger.AssertNoOutputMessages(); } [TestMethod] - public void RemoveNotification_HasACurrentNotification_RemovesCurrentNotification() + public void CloseNotification_HasACurrentNotification_RemovesCurrentNotification() { - Check_HasACurrentNotification_RemovesCurrentNotification(x => x.RemoveNotification()); + Check_HasACurrentNotification_RemovesCurrentNotification(x => x.CloseNotification()); } [TestMethod] - public void RemoveNotification_CurrentNotificationIsNull_DoesNotThrow() + public void CloseNotification_CurrentNotificationIsNull_DoesNotThrow() { var infoBarManager = new Mock(); var testSubject = CreateTestSubject(infoBarManager.Object); - Action act = () => testSubject.RemoveNotification(); + Action act = () => testSubject.CloseNotification(); act.Should().NotThrow(); @@ -539,13 +476,13 @@ public void RemoveNotification_CurrentNotificationIsNull_DoesNotThrow() } [TestMethod] - public void RemoveNotification_InfoBarRemovedIsCalledOnMainThread() + public void CloseNotification_InfoBarRemovedIsCalledOnMainThread() { // Arrange var calls = new List(); var notification = CreateNotification(); - var infoBarManager = CreateInfoBarManager(notification, CreateInfoBar().Object); + var infoBarManager = CreateInfoBarManager(notification, CreateInfoBar()); infoBarManager.Setup(x => x.CloseInfoBar(It.IsAny())) .Callback(x => calls.Add("CloseInfoBar")); @@ -565,18 +502,18 @@ public void RemoveNotification_InfoBarRemovedIsCalledOnMainThread() calls.Clear(); // Act - testSubject.RemoveNotification(); + testSubject.CloseNotification(); calls.Should().ContainInOrder("RunOnUIThread", "CloseInfoBar"); } [TestMethod] - public void Dispose_NonCriticalException_ExceptionCaught() + public void CloseNotification_NonCriticalException_ExceptionCaught() { var notification = CreateNotification(); var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); infoBarManager .Setup(x => x.CloseInfoBar(infoBar.Object)) .Throws(new NotImplementedException("this is a test")); @@ -586,19 +523,19 @@ public void Dispose_NonCriticalException_ExceptionCaught() var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger); testSubject.ShowNotification(notification); - Action act = () => testSubject.Dispose(); + Action act = () => testSubject.CloseNotification(); act.Should().NotThrow(); logger.AssertPartialOutputStringExists("this is a test"); } [TestMethod] - public void Dispose_CriticalException_ExceptionNotCaught() + public void CloseNotification_CriticalException_ExceptionNotCaught() { var notification = CreateNotification(); var infoBar = CreateInfoBar(); - var infoBarManager = CreateInfoBarManager(notification, infoBar.Object); + var infoBarManager = CreateInfoBarManager(notification, infoBar); infoBarManager .Setup(x => x.CloseInfoBar(infoBar.Object)) .Throws(new StackOverflowException("this is a test")); @@ -608,7 +545,7 @@ public void Dispose_CriticalException_ExceptionNotCaught() var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger); testSubject.ShowNotification(notification); - Action act = () => testSubject.Dispose(); + Action act = () => testSubject.CloseNotification(); act.Should().ThrowExactly().WithMessage("this is a test"); logger.AssertNoOutputMessages(); @@ -618,7 +555,7 @@ private void Check_HasACurrentNotification_RemovesCurrentNotification(Action CreateInfoBar() return infoBar; } - private static Mock CreateInfoBarManager(INotification notification, IInfoBar infoBar) + private static Mock CreateInfoBarManager(INotification notification, Mock infoBar) { var infoBarManager = new Mock(); @@ -681,14 +618,15 @@ private static Mock CreateInfoBarManager(INotification notifica return infoBarManager; } - private static void SetupInfoBarManager(Mock infoBarManager, INotification notification, IInfoBar infoBar) + private static void SetupInfoBarManager(Mock infoBarManager, INotification notification, Mock infoBar) { infoBarManager .Setup(x => x.AttachInfoBarToMainWindow( notification.Message, SonarLintImageMoniker.OfficialSonarLintMoniker, It.IsAny())) - .Returns(infoBar); + .Returns(infoBar.Object); + infoBarManager.Setup(x => x.CloseInfoBar(infoBar.Object)).Callback(() => infoBar.Raise(x => x.Closed += null, EventArgs.Empty)); } private static void VerifyInfoBarCreatedCorrectly(Mock infoBarManager, INotification notification) diff --git a/src/Core/Notifications/INotificationService.cs b/src/Core/Notifications/INotificationService.cs index 6c4cb1168f..a055aa7e48 100644 --- a/src/Core/Notifications/INotificationService.cs +++ b/src/Core/Notifications/INotificationService.cs @@ -35,7 +35,7 @@ public interface INotificationService : IDisposable { void ShowNotification(INotification notification); - void RemoveNotification(); + void CloseNotification(); } [Export(typeof(INotificationService))] @@ -86,12 +86,30 @@ public void ShowNotification(INotification notification) threadHandling.RunOnUIThreadAsync(() => { - RemoveExistingInfoBar(); + CloseNotification(); ShowInfoBar(notification); }); } - public void RemoveNotification() => RemoveExistingInfoBar(); + public void CloseNotification() + { + if (activeNotification == null) + { + return; + } + + threadHandling.RunOnUIThreadAsync(() => + { + try + { + infoBarManager.CloseInfoBar(activeNotification.Item1); + } + catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex)) + { + logger.WriteLine(CoreStrings.Notifications_FailedToRemove, ex); + } + }); + } private void CurrentInfoBar_ButtonClick(object sender, InfoBarButtonClickedEventArgs e) { @@ -104,7 +122,7 @@ private void CurrentInfoBar_ButtonClick(object sender, InfoBarButtonClickedEvent if (matchingAction?.ShouldDismissAfterAction == true) { - RemoveExistingInfoBar(); + CloseNotification(); } } catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex)) @@ -114,31 +132,15 @@ private void CurrentInfoBar_ButtonClick(object sender, InfoBarButtonClickedEvent } private void CurrentInfoBar_Closed(object sender, EventArgs e) - { - RemoveExistingInfoBar(); - } - - private void RemoveExistingInfoBar() { if (activeNotification == null) { return; } - threadHandling.RunOnUIThreadAsync(() => - { - try - { - activeNotification.Item1.ButtonClick -= CurrentInfoBar_ButtonClick; - activeNotification.Item1.Closed -= CurrentInfoBar_Closed; - infoBarManager.CloseInfoBar(activeNotification.Item1); - activeNotification = null; - } - catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex)) - { - logger.WriteLine(CoreStrings.Notifications_FailedToRemove, ex); - } - }); + activeNotification.Item1.ButtonClick -= CurrentInfoBar_ButtonClick; + activeNotification.Item1.Closed -= CurrentInfoBar_Closed; + activeNotification = null; } private void ShowInfoBar(INotification notification) @@ -167,7 +169,7 @@ private void ShowInfoBar(INotification notification) public void Dispose() { - RemoveExistingInfoBar(); + CloseNotification(); } } } diff --git a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionNotificationTests.cs b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionNotificationTests.cs index ffb160f232..8d8fde543a 100644 --- a/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionNotificationTests.cs +++ b/src/IssueViz.UnitTests/FixSuggestion/FixSuggestionNotificationTests.cs @@ -191,7 +191,7 @@ public void Clear_TriggersNotificationServiceClear() { testSubject.Clear(); - notificationService.Received(1).RemoveNotification(); + notificationService.Received(1).CloseNotification(); } private void AssertReceivedNotificationWithMessage(string message) diff --git a/src/IssueViz/FixSuggestion/IFixSuggestionNotification.cs b/src/IssueViz/FixSuggestion/IFixSuggestionNotification.cs index 4e93d55ad2..11f89b75f7 100644 --- a/src/IssueViz/FixSuggestion/IFixSuggestionNotification.cs +++ b/src/IssueViz/FixSuggestion/IFixSuggestionNotification.cs @@ -84,7 +84,7 @@ private void Show(string text) public void Clear() { - notificationService.RemoveNotification(); + notificationService.CloseNotification(); } public void Dispose()