-
Notifications
You must be signed in to change notification settings - Fork 77
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.
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
SLVS-1406 Add binding changed trigger to ActiveSolutionBoundTracker #5642
SLVS-1406 Add binding changed trigger to ActiveSolutionBoundTracker #5642
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some feedback regarding the test class.
eventCounter.SolutionBindingChangedCount.Should().Be(0); | ||
|
||
eventCounter.RaisedEventNames.Should().HaveCount(2); | ||
eventCounter.RaisedEventNames[0].Should().Be("PreSolutionBindingUpdated"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use name of instead of hardcoded string for event name.
Both in here and in the line 626
eventCounter.SolutionBindingUpdatedCount.Should().Be(0); | ||
|
||
eventCounter.RaisedEventNames.Should().HaveCount(2); | ||
eventCounter.RaisedEventNames[0].Should().Be("PreSolutionBindingChanged"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use name of instead of hardcoded string for event name.
Maybe search and replace in the whole file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't change these, but I can try to fix this.
} | ||
|
||
[TestMethod] | ||
public void HandleBindingChange_NewConfiguration_EventsRaisedInCorrectOrder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test seems to be a copy-past of OnBindingStateChanged_NewConfiguration_EventsRaisedInCorrectOrder.
Can we maybe extract the whole content in a method and provide the act as a parameter to the method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The event-based test will be removed once we get rid of the old binding window
|
||
[TestMethod] | ||
public void HandleBindingChange_WhenClearBoundProject_NotRaised() | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test seems to be a copy-past of SolutionBindingUpdated_WhenClearBoundProject_NotRaised.
Can we maybe extract the whole content in a method and provide the act as a parameter to the method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The event-based test will be removed once we get rid of the old binding window
@@ -544,6 +603,29 @@ public void SolutionBindingUpdated_WhenSetBoundProject_EventsRaisedInExpectedOrd | |||
eventCounter.RaisedEventNames[1].Should().Be("SolutionBindingUpdated"); | |||
} | |||
} | |||
|
|||
[TestMethod] | |||
public void HandleBindingChange_WhenSetBoundProject_EventsRaisedInExpectedOrder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test seems to be a copy-past of SolutionBindingUpdated_WhenSetBoundProject_EventsRaisedInExpectedOrder.
Can we maybe extract the whole content in a method and provide the act as a parameter to the method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The event-based test will be removed once we get rid of the old binding window
Quality Gate passedIssues Measures |
ccf0822
into
feature/new-connected-mode
SLVS-1406