-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5598: Fix CoreServerSessionPool.IsAboutToExpire test #1699
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
public void IsAboutToExpire_should_never_expire_in_load_balancing_mode() | ||
{ | ||
var subject = CreateSubject(); | ||
var mockedCluster = new TestCluster(ClusterType.LoadBalanced); |
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.
In scope of CSOT I've updated IClusterInternal
interface, instead of mirroring the changes here, I've decided to remove local implementation by using Moq
(CreateSubject
method already constructing that mock)
var mockedCluster = new TestCluster(ClusterType.LoadBalanced); | ||
var mockedServerSessionPool = new CoreServerSessionPool(mockedCluster); | ||
var mockSession = new Mock<ICoreServerSession>(); | ||
var lastUsedAt = DateTime.UtcNow.AddSeconds(1741); |
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 line of code is incorrect, we should add negative seconds to make session looks expired (see line 231 before the changes).
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.
LGTM + minor comment
[InlineData(ClusterType.LoadBalanced, null, false)] | ||
[InlineData(ClusterType.LoadBalanced, 1741, false)] | ||
[InlineData(ClusterType.LoadBalanced, 1739, false)] | ||
public void IsAboutToExpire_should_return_expected_result(ClusterType clusterType, int? lastUsedSecondsAgo, bool expectedResult) |
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.
nit: rename expectedResult
to isAboutToExpire
?
No description provided.