-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix test for random initialized node #4134
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.
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. I was intentionally trying to avoid going this route, because it now feels like it is more testing that Random
seeded with different seed values provide nodes in randomized order 😄 I think however, this is probably the right seam to introduce, although I think we should consider accepting an instance of Random
as the ctor argument in master.
@@ -15,27 +15,35 @@ public StaticConnectionPool(IEnumerable<Uri> uris, bool randomize = true, IDateT | |||
: this(uris.Select(uri => new Node(uri)), randomize, dateTimeProvider) { } | |||
|
|||
public StaticConnectionPool(IEnumerable<Node> nodes, bool randomize = true, IDateTimeProvider dateTimeProvider = null) | |||
: this(nodes, randomize, randomizeSeed: null, dateTimeProvider) { } | |||
|
|||
protected StaticConnectionPool(IEnumerable<Node> nodes, bool randomize, int? randomizeSeed = null, IDateTimeProvider dateTimeProvider = null) |
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 wonder if for master, we should consider accepting an instance of Random
here instead of randomize
and randomizeSeed
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)
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)
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)
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.
Continuation of #4112