Capture variables in tests and add logging. #942
Conversation
|
My suggestion would be to remove |
| @@ -42,13 +43,15 @@ func TestRateLimiterSingleThreaded(t *testing.T) { | |||
| if qps > float64(limit)*1.01 { | |||
| t.Errorf("#%d: Too many operations per second. Expected ~%d, got %f", i, limit, qps) | |||
| } | |||
| log.Infof("#%d: Expected ~%d, got %f", i, limit, qps) | |||
There was a problem hiding this comment.
I think this is redundant. The expected value is part of the test name, and the qps only needs printing if it's not matching the expected value, which already happens 2 lines above.
Same below.
| @@ -26,6 +26,7 @@ var testlimits = []int{1, 10, 50, 100, 1000} | |||
| func TestRateLimiterSingleThreaded(t *testing.T) { | |||
| for i, limit := range testlimits { | |||
| t.Run(fmt.Sprintf("%d ops/s", limit), func(t *testing.T) { | |||
| i, limit := i, limit | |||
|
+1 for removing them altogether. I have observed unexpected test failures where more than 2k operations per seconds happened, when there should never be more than 1k. Given the unpredictable behaviour of the current test code, I cannot tell for sure if it's because we're using |
Capture variable in tests to execute them all. It also adds logging to see how tests have performed.
Checklist