-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix test for random initialized node (#4134) #4144
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
Conversation
The test was using a local method that yielded a single pool while later .Take(50) was requested on this sequence. This always yields an enumeration with 1 element which increases the chances for the assertion to turn false. Rather than relying on Thread.Sleep to create different seeded randoms the StaticConnectionPool now defines a protected constructor that allows one to pass in a seed value. (cherry picked from commit 9734f04)
port to master of #4134 |
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 think two properties gained public setters again inadvertently other than that LGTM, thanks for doing the forward port!
@@ -62,7 +77,7 @@ protected StaticConnectionPool(IEnumerable<Node> nodes, Func<Node, float> nodeSc | |||
public virtual bool SupportsReseeding => false; | |||
|
|||
/// <inheritdoc /> | |||
public bool UsingSsl { get; } | |||
public bool UsingSsl { get; set; } |
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.
public bool UsingSsl { get; set; } | |
public bool UsingSsl { get; } |
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.
Should be a private setter
@@ -75,10 +90,10 @@ protected List<Node> AliveNodes | |||
} | |||
} | |||
|
|||
protected IDateTimeProvider DateTimeProvider { get; } | |||
protected IDateTimeProvider DateTimeProvider { get; set; } |
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.
protected IDateTimeProvider DateTimeProvider { get; set; } | |
protected IDateTimeProvider DateTimeProvider { get; } |
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.
Should be a private setter
Would you mind taking a look again @Mpdreamz |
@russcam compilation failed on CI |
Fat-fingered a 'B' when running a build with CTRL + SHIFT + b...
D'oh, fat fingered a |
Merging this in; failing tests are not related to this change, |
The test was using a local method that yielded a single pool while later .Take(50)
was requested on this sequence. This always yields an enumeration with 1 element
which increases the chances for the assertion to turn false.
Rather than relying on Thread.Sleep to create different seeded randoms the StaticConnectionPool
now defines a protected constructor that allows one to pass in a seed value.
(cherry picked from commit 9734f04)