search

kubernetes-retired / service-catalog

Consume services in Kubernetes using the Open Service Broker API
https://svc-cat.io
Apache License 2.0
1.05k stars 385 forks source link

Add timeouts to all the tests #1324

Closed vaikas closed 6 years ago

vaikas commented 7 years ago

Due to a bug in the code, our tests were hanging forever and needed a manual abort. https://service-catalog-jenkins.appspot.com/job/service-catalog-PR-testing2/2398/

It was basically waiting for a ServiceClass to be ready looked like. If you revert this commit it will fail: https://github.com/kubernetes-incubator/service-catalog/pull/1319/commits/87ba116213aabee6eeecb738b85e904ca59e2e34

pmorie commented 7 years ago

0.1.1 candidate

MHBauer commented 6 years ago

@vaikas-google @kibbles-n-bytes Are we still seeing hangs? Does this need to be worked?

n3wscott commented 6 years ago

I agree we still need the timeout added to the tests

kibbles-n-bytes commented 6 years ago

Yes; I added a timeout to Jenkins as a result of this issue to abort the entire build, but I believe we should have timeouts per test.

MHBauer commented 6 years ago

For integration and e2e, we pass the t *testing.T into the framework, so maybe setting a timer goroutine on t.Fatalf would work. I think the ones involving the framework are probably most likely to take long enough to need a timeout.

There doesn't seem to be a built-in way to add a timeout to a test. We'd have to write a timeoutFailer and added it to all the tests.

MHBauer commented 6 years ago

Don't see anyway to do this in a manner that cleanly moves on to the next test. It would require plumbing through the stopch through every test and then checking if it's time to stop.

The other option is to accept that it's timed out and panic and stop the whole process.

MHBauer commented 6 years ago

prow has timeouts per job so this should be solved by having moved most of the test to prow. /close