-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add Graphite Integration Test #1758
Conversation
Looking good @corylanou -- badly needed. |
446ae53
to
cbf4057
Compare
@@ -50,9 +50,14 @@ func Run(config *Config, join, version string, logWriter *os.File) (*messaging.B | |||
// We want to make sure we are spun up before we exit this function, so we manually listen and serve | |||
listener, err := net.Listen("tcp", config.BrokerAddr()) | |||
if err != nil { | |||
log.Fatal(err) | |||
log.Fatalf("failed to listen for broker on tcp: %s ", err) |
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.
Minor suggestion: include the value of config.BrokerAddr()
in the log message. It might be better phrased as something like "Broker failed to listen on %s: %s" where the first variable is the address, the second is the error. Brokers only listen over TCP, so mentioning TCP isn't strictly necessary.
10c18ed
to
8a9a635
Compare
@corylanou is this guy ready to go? |
This test is very brittle, just so you know. |
It's why I haven't merged it yet :-) It's only brittle because of a bug that is in the broker that Ben is currently fixing. I'm going to wait until he has that fixed and then update all the tests to re-enable index/wait endpoints. This will significantly speed up the integration tests, which are now taking almost a minute to run. |
(continues to wait patiently) |
8a9a635
to
6a05d03
Compare
6a05d03
to
98484b6
Compare
Fixed at brittle test methodology. We now have a wait that will poll for expected results, or times out. Ready for review. |
@@ -167,9 +173,6 @@ func write(t *testing.T, node *Node, data string) { | |||
body, _ := ioutil.ReadAll(resp.Body) | |||
t.Fatalf("Write to database failed. Unexpected status code. expected: %d, actual %d, %s", http.StatusOK, resp.StatusCode, string(body)) | |||
} | |||
|
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.
Legend!
You are adding a file called |
} else if atomic.LoadInt32(&timedOut) == 1 { | ||
return fmt.Sprintf("timed out before expected result was found: got: %s", got), false | ||
} else { | ||
time.Sleep(100 * time.Millisecond) |
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 see no reason why this can't be 10 milliseconds.
+1 |
Add Graphite Integration Test
This PR is to address issue ##1735.