-
Notifications
You must be signed in to change notification settings - Fork 595
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
Protect against ping periods that are out of range #488
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package coordinate | ||
|
||
import ( | ||
"fmt" | ||
"math" | ||
"reflect" | ||
"strings" | ||
|
@@ -64,6 +65,60 @@ func TestClient_Update(t *testing.T) { | |
verifyEqualFloats(t, c.Vec[2], 99.0) | ||
} | ||
|
||
func TestClient_InvalidInPingValues(t *testing.T) { | ||
config := DefaultConfig() | ||
config.Dimensionality = 3 | ||
|
||
client, err := NewClient(config) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Make sure the Euclidean part of our coordinate is what we expect. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This upper part doesn't seem necessary since we test the coordinate client elsewhere. I'd just try to update with a negative and a too large value and make sure you get the expected error back. |
||
c := client.GetCoordinate() | ||
verifyEqualVectors(t, c.Vec, []float64{0.0, 0.0, 0.0}) | ||
|
||
// Place another node | ||
other := NewCoordinate(config) | ||
other.Vec[2] = 0.001 | ||
rtt := time.Duration(2.0 * other.Vec[2] * secondsToNanoseconds) | ||
c, err = client.Update("node", other, rtt) | ||
if err != nil { | ||
t.Fatalf("err: %v", err) | ||
} | ||
|
||
dist_old := client.DistanceTo(other) | ||
// Update with a valid ping period, should have an affect on estimated rtt | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "effect" |
||
ping := 20 | ||
_, err = client.Update("node", other, time.Duration(ping*secondsToNanoseconds)) | ||
if err != nil { | ||
t.Fatalf("Unexpected error ", err) | ||
} | ||
|
||
dist_new := client.DistanceTo(other) | ||
if dist_new <= dist_old { | ||
t.Fatalf("Invalid rtt estimate %v", dist_new) | ||
} | ||
|
||
// Update with a series of invalid ping periods, should return an error and estimated rtt remains unchanged | ||
pings := []int{1<<63 - 1, -35, -1 << 63, 35000} | ||
dist_old = dist_new | ||
|
||
for _, ping := range pings { | ||
expectedErr := fmt.Errorf("Round trip time not in valid range, duration %v is not a positive value less than %v", ping, MAX_RTT_SECONDS) | ||
_, err = client.Update("node", other, time.Duration(ping*secondsToNanoseconds)) | ||
if err == nil { | ||
t.Fatalf("Unexpected error, wanted %v but got %v", expectedErr, err) | ||
} | ||
|
||
dist_new = client.DistanceTo(other) | ||
if dist_new != dist_old { | ||
t.Fatalf("distance est", dist_new) | ||
} | ||
} | ||
|
||
} | ||
|
||
func TestClient_DistanceTo(t *testing.T) { | ||
config := DefaultConfig() | ||
config.Dimensionality = 3 | ||
|
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.
You can do this as a
time.Duration
(I'd put the constant right here, too):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'd also rail it way lower since 10 seconds is already several times around the Earth, so this should keep the coordinates better behaved in the face of weirdness (and it's not useful to model anything slower).