-
Notifications
You must be signed in to change notification settings - Fork 164
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
Reduce overall testing time of juno through changes in REST tests #256
Comments
Maybe we add a I haven't explored this much, but wanted to post it here just in case the idea is useful. |
I think this issue is a bit more involved than I expected as the timeout is set at the client level, and there is no easy place to change this. Would yoiu like to have a go with your approach @joshklop ? |
I can give it a try, but I may not be able to get to it for a bit. |
Scope might have changed: It would be good to use the httptest standard library exclusively, instead of relying on external dependencies |
Hi guys! I was working on another PR and as I was running all the tests, I also saw that the tests within rest_test.go were very slow. I looked at it and the problem is that the http client timeout will never be taken into account as you have an hardcoded time.sleep in you code here: pkg/feeder/feeder.go:129. You need to extract the default retry function (the loop) implemented there without breaking the current implementation used by other functions. What you could do is allowing to pass a retry function as parameter. Something like that:
This would allow you to pass a retry function from the tests and modify the sleep time and so by extention the timeout time. Happy to open a PR if you guys want. |
That looks awesome @v4lproik! Thank you so much. A PR would be great. If you need anything please let me know. |
I decided to extend the client so it takes a retry mechanism function as parameter in NewClientWithRetryFuncForDoReq constructor. If the default constructor is being used then I initiate the retry function with the same piece of code that was hardcoded in the Do function before. Hope that helps. |
Tests currently take a long time because the rest api tests which are meant to fail (within /pkg/rest/tests/rest_test.go) take a long time to timeout.
This means that, while the rest of the test suite finishes in less than 10 seconds, all the Test(...)Fail take ~ 10s each, 13 times, incrementing this substantially and wasting time.
It would be good to have 1 timeout test with the normal rate, and the other failing tests to have a much smaller timeout or use a different method to check for failure than to wait out the full 10s.
The text was updated successfully, but these errors were encountered: