Arvados

2014/10/06 14:10:57 TestGetDisabled start
2014/10/06 14:10:57 keepclient created

----------------------------------------------------------------------
PANIC: /tmp/tmp.O0vXh97Iqx/src/git.curoverse.com/arvados.git/services/keepproxy/keepproxy_test.go:239: ServerRequiredSuite.TestGetDisabled

... Panic: runtime error: invalid memory address or nil pointer dereference (PC=0x414086)

/usr/local/go/src/pkg/runtime/panic.c:248
  in panic
/usr/local/go/src/pkg/runtime/panic.c:482
  in panicstring
/usr/local/go/src/pkg/runtime/os_linux.c:234
  in sigpanic
/tmp/tmp.O0vXh97Iqx/src/git.curoverse.com/arvados.git/services/keepproxy/keepproxy_test.go:243
  in ServerRequiredSuite.TestGetDisabled

review @ commit d9166dfa

There may be a clever way to do this more reliably without exposing ugly test harness stuff, but it's not obvious. For our purposes, this seems like a fine improvement over the status quo, so LGTM.

Going forward, I think it would help Keepstore and Keepproxy (and probably Data Manager) development to have comprehensive mock/fake interfaces for Keepstore and apiserver, so any of these packages' test suites can just instantiate a FakeKeepstore or FakeApiServer that runs in a thread and responds to requests with canned responses. The kind of integration test we are doing here is useful, but it would be nice to test as much as possible in a more deterministic environment.

Tim Pierce wrote:

There may be a clever way to do this more reliably without exposing ugly test harness stuff, but it's not obvious. For our purposes, this seems like a fine improvement over the status quo, so LGTM.

Surely it's possible to put hooks into http.Serve() using testing and/or gomock, but for now, "server isn't listening within 1 second" is arguably a real bug anyway. :)

Going forward, I think it would help Keepstore and Keepproxy (and probably Data Manager) development to have comprehensive mock/fake interfaces for Keepstore and apiserver, so any of these packages' test suites can just instantiate a FakeKeepstore or FakeApiServer that runs in a thread and responds to requests with canned responses. The kind of integration test we are doing here is useful, but it would be nice to test as much as possible in a more deterministic environment.

I agree, mocking the back-end services will let us test more stuff -- like appropriate behavior when back-end services misbehave -- and give us a much quicker test suite.

(Kind of funny that the Keepproxy-under-test itself, rather than the run_test_server stuff, is the source of this server-not-ready race...)