Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SLVS-1508 Closing a goldbar notification results in editor freeze #5750

Merged
merged 4 commits into from
Oct 16, 2024
Merged

SLVS-1508 Closing a goldbar notification results in editor freeze #5750

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4eca7be
Separate close notification from notification cleanup
vnaskos-sonar Oct 15, 2024
25ac88c
Rename RemoveNotification to CloseNotification for clarity
vnaskos-sonar Oct 15, 2024
afb8966
Update tests
vnaskos-sonar Oct 15, 2024
407f75b
Refactor tests that were testing CloseNotification behavior on Dispos…
vnaskos-sonar Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 28 additions & 90 deletions src/Core.UnitTests/Notifications/NotificationServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<IThreadHandling>();
Action runOnUiAction = null;
Expand Down Expand Up @@ -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);
Expand All @@ -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<IDisabledNotificationsStorage>();
disabledNotificationsStorage.Setup(x => x.IsNotificationDisabled("some id")).Returns(true);
Expand All @@ -137,8 +137,8 @@ public void ShowNotification_NotificationIsDisabled_PreviousNotificationNotRemov
var infoBar2 = CreateInfoBar();

var infoBarManager = new Mock<IInfoBarManager>();
SetupInfoBarManager(infoBarManager, notification1, infoBar1.Object);
SetupInfoBarManager(infoBarManager, notification2, infoBar2.Object);
SetupInfoBarManager(infoBarManager, notification1, infoBar1);
SetupInfoBarManager(infoBarManager, notification2, infoBar2);

var disabledNotificationsStorage = new Mock<IDisabledNotificationsStorage>();
disabledNotificationsStorage.Setup(x => x.IsNotificationDisabled("some id1")).Returns(false);
Expand Down Expand Up @@ -178,9 +178,9 @@ public void ShowNotification_OncePerSessionEnaled_NotificationWithSameIdAlreadyS
var infoBar3 = CreateInfoBar();

var infoBarManager = new Mock<IInfoBarManager>();
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);

Expand Down Expand Up @@ -215,9 +215,9 @@ public void ShowNotification_OncePerSessionDisabled_NotificationWithSameIdAlread
var infoBar3 = CreateInfoBar();

var infoBarManager = new Mock<IInfoBarManager>();
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);

Expand Down Expand Up @@ -251,7 +251,7 @@ public void ShowNotification_UnknownInfoBarButtonClicked_NoException()

var infoBar = new Mock<IInfoBar>();

var infoBarManager = CreateInfoBarManager(notification, infoBar.Object);
var infoBarManager = CreateInfoBarManager(notification, infoBar);

var testSubject = CreateTestSubject(infoBarManager.Object);
testSubject.ShowNotification(notification);
Expand All @@ -277,7 +277,7 @@ public void ShowNotification_InfoBarButtonClicked_ActionInvokedWithTheNotificati

var infoBar = new Mock<IInfoBar>();

var infoBarManager = CreateInfoBarManager(notification, infoBar.Object);
var infoBarManager = CreateInfoBarManager(notification, infoBar);

var testSubject = CreateTestSubject(infoBarManager.Object);
testSubject.ShowNotification(notification);
Expand Down Expand Up @@ -305,7 +305,7 @@ public void ShowNotification_InfoBarButtonClicked_CorrectActionInvoked()

var infoBar = new Mock<IInfoBar>();

var infoBarManager = CreateInfoBarManager(notification, infoBar.Object);
var infoBarManager = CreateInfoBarManager(notification, infoBar);

var testSubject = CreateTestSubject(infoBarManager.Object);
testSubject.ShowNotification(notification);
Expand Down Expand Up @@ -333,8 +333,8 @@ public void ShowNotification_HasPreviousNotification_PreviousNotificationRemoved
var infoBar2 = CreateInfoBar();

var infoBarManager = new Mock<IInfoBarManager>();
SetupInfoBarManager(infoBarManager, notification1, infoBar1.Object);
SetupInfoBarManager(infoBarManager, notification2, infoBar2.Object);
SetupInfoBarManager(infoBarManager, notification1, infoBar1);
SetupInfoBarManager(infoBarManager, notification2, infoBar2);

var testSubject = CreateTestSubject(infoBarManager.Object);

Expand All @@ -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()
{
Expand Down Expand Up @@ -442,7 +422,7 @@ public void ShowNotification_InfoBarButtonClicked_NonCriticalException_Exception

var infoBar = new Mock<IInfoBar>();

var infoBarManager = CreateInfoBarManager(notification, infoBar.Object);
var infoBarManager = CreateInfoBarManager(notification, infoBar);
var logger = new TestLogger();

var testSubject = CreateTestSubject(infoBarManager.Object, logger: logger);
Expand All @@ -454,56 +434,13 @@ public void ShowNotification_InfoBarButtonClicked_NonCriticalException_Exception
logger.AssertPartialOutputStringExists("this is a test");
}

[TestMethod]
vnaskos-sonar marked this conversation as resolved.
Show resolved Hide resolved
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<IInfoBar>();

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<StackOverflowException>().WithMessage("this is a test");
logger.AssertNoOutputMessages();
}

[TestMethod]
public void ShowNotification_UserClosedTheNotification_NonCriticalException_ExceptionCaught()
vnaskos-sonar marked this conversation as resolved.
Show resolved Hide resolved
{
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"));
Expand All @@ -512,8 +449,8 @@ 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<StackOverflowException>().WithMessage("this is a test");

logger.AssertNoOutputMessages();
Expand Down Expand Up @@ -545,7 +482,7 @@ public void CloseNotification_InfoBarRemovedIsCalledOnMainThread()
var calls = new List<string>();

var notification = CreateNotification();
var infoBarManager = CreateInfoBarManager(notification, CreateInfoBar().Object);
var infoBarManager = CreateInfoBarManager(notification, CreateInfoBar());
infoBarManager.Setup(x => x.CloseInfoBar(It.IsAny<IInfoBar>()))
.Callback<IInfoBar>(x => calls.Add("CloseInfoBar"));

Expand Down Expand Up @@ -576,7 +513,7 @@ public void Dispose_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"));
Expand All @@ -598,7 +535,7 @@ public void Dispose_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"));
Expand All @@ -618,7 +555,7 @@ private void Check_HasACurrentNotification_RemovesCurrentNotification(Action<Not
{
var notification = CreateNotification();
var infoBar = CreateInfoBar();
var infoBarManager = CreateInfoBarManager(notification, infoBar.Object);
var infoBarManager = CreateInfoBarManager(notification, infoBar);
var testSubject = CreateTestSubject(infoBarManager.Object);

testSubject.ShowNotification(notification);
Expand Down Expand Up @@ -672,7 +609,7 @@ private static Mock<IInfoBar> CreateInfoBar()
return infoBar;
}

private static Mock<IInfoBarManager> CreateInfoBarManager(INotification notification, IInfoBar infoBar)
private static Mock<IInfoBarManager> CreateInfoBarManager(INotification notification, Mock<IInfoBar> infoBar)
{
var infoBarManager = new Mock<IInfoBarManager>();

Expand All @@ -681,14 +618,15 @@ private static Mock<IInfoBarManager> CreateInfoBarManager(INotification notifica
return infoBarManager;
}

private static void SetupInfoBarManager(Mock<IInfoBarManager> infoBarManager, INotification notification, IInfoBar infoBar)
private static void SetupInfoBarManager(Mock<IInfoBarManager> infoBarManager, INotification notification, Mock<IInfoBar> infoBar)
{
infoBarManager
.Setup(x => x.AttachInfoBarToMainWindow(
notification.Message,
SonarLintImageMoniker.OfficialSonarLintMoniker,
It.IsAny<string[]>()))
.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<IInfoBarManager> infoBarManager, INotification notification)
Expand Down