5824: Merge branch 'master' into 5824-keep-web-workbench

Conflicts:
	services/keepproxy/keepproxy_test.go

diff --combined sdk/go/arvadosclient/arvadosclient_test.go
index d1487235dcf6f71033e7fb8232bf6a8955d84d35,f2fd0b1b334a3e3790e6bb8c39a60db8a335ad66..8e32efe4f987adccbe77b573a15ab27dbdfcc707
--- 1/sdk/go/arvadosclient/arvadosclient_test.go
--- 2/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@@ -1,8 -1,10 +1,10 @@@
  package arvadosclient
  
  import (
+       "fmt"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        . "gopkg.in/check.v1"
+       "net"
        "net/http"
        "os"
        "testing"
@@@ -16,6 -18,7 +18,7 @@@ func Test(t *testing.T) 
  
  var _ = Suite(&ServerRequiredSuite{})
  var _ = Suite(&UnitSuite{})
+ var _ = Suite(&MockArvadosServerSuite{})
  
  // Tests that require the Keep server running
  type ServerRequiredSuite struct{}
@@@ -23,13 -26,9 +26,14 @@@
  func (s *ServerRequiredSuite) SetUpSuite(c *C) {
        arvadostest.StartAPI()
        arvadostest.StartKeep(2, false)
+       RetryDelay = 0
  }
  
 +func (s *ServerRequiredSuite) TearDownSuite(c *C) {
 +      arvadostest.StopKeep(2)
 +      arvadostest.StopAPI()
 +}
 +
  func (s *ServerRequiredSuite) SetUpTest(c *C) {
        arvadostest.ResetEnv()
  }
@@@ -223,3 -222,160 +227,160 @@@ func (s *UnitSuite) TestPDHMatch(c *C) 
        c.Assert(PDHMatch("+12345"), Equals, false)
        c.Assert(PDHMatch(""), Equals, false)
  }
+ 
+ // Tests that use mock arvados server
+ type MockArvadosServerSuite struct{}
+ 
+ func (s *MockArvadosServerSuite) SetUpSuite(c *C) {
+       RetryDelay = 0
+ }
+ 
+ func (s *MockArvadosServerSuite) SetUpTest(c *C) {
+       arvadostest.ResetEnv()
+ }
+ 
+ type APIServer struct {
+       listener net.Listener
+       url      string
+ }
+ 
+ func RunFakeArvadosServer(st http.Handler) (api APIServer, err error) {
+       api.listener, err = net.ListenTCP("tcp", &net.TCPAddr{Port: 0})
+       if err != nil {
+               return
+       }
+       api.url = api.listener.Addr().String()
+       go http.Serve(api.listener, st)
+       return
+ }
+ 
+ type APIStub struct {
+       method        string
+       retryAttempts int
+       expected      int
+       respStatus    []int
+       responseBody  []string
+ }
+ 
+ func (h *APIStub) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
+       if req.URL.Path == "/redirect-loop" {
+               http.Redirect(resp, req, "/redirect-loop", http.StatusFound)
+               return
+       }
+       if h.respStatus[h.retryAttempts] < 0 {
+               // Fail the client's Do() by starting a redirect loop
+               http.Redirect(resp, req, "/redirect-loop", http.StatusFound)
+       } else {
+               resp.WriteHeader(h.respStatus[h.retryAttempts])
+               resp.Write([]byte(h.responseBody[h.retryAttempts]))
+       }
+       h.retryAttempts++
+ }
+ 
+ func (s *MockArvadosServerSuite) TestWithRetries(c *C) {
+       for _, stub := range []APIStub{
+               {
+                       "get", 0, 200, []int{200, 500}, []string{`{"ok":"ok"}`, ``},
+               },
+               {
+                       "create", 0, 200, []int{200, 500}, []string{`{"ok":"ok"}`, ``},
+               },
+               {
+                       "get", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "create", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "update", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "delete", 0, 500, []int{500, 500, 500, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "get", 0, 502, []int{500, 500, 502, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "create", 0, 502, []int{500, 500, 502, 200}, []string{``, ``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "get", 0, 200, []int{500, 500, 200}, []string{``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "create", 0, 200, []int{500, 500, 200}, []string{``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "delete", 0, 200, []int{500, 500, 200}, []string{``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "update", 0, 200, []int{500, 500, 200}, []string{``, ``, `{"ok":"ok"}`},
+               },
+               {
+                       "get", 0, 401, []int{401, 200}, []string{``, `{"ok":"ok"}`},
+               },
+               {
+                       "create", 0, 401, []int{401, 200}, []string{``, `{"ok":"ok"}`},
+               },
+               {
+                       "get", 0, 404, []int{404, 200}, []string{``, `{"ok":"ok"}`},
+               },
+               {
+                       "get", 0, 401, []int{500, 401, 200}, []string{``, ``, `{"ok":"ok"}`},
+               },
+ 
+               // Response code -1 simulates an HTTP/network error
+               // (i.e., Do() returns an error; there is no HTTP
+               // response status code).
+ 
+               // Succeed on second retry
+               {
+                       "get", 0, 200, []int{-1, -1, 200}, []string{``, ``, `{"ok":"ok"}`},
+               },
+               // "POST" is not safe to retry: fail after one error
+               {
+                       "create", 0, -1, []int{-1, 200}, []string{``, `{"ok":"ok"}`},
+               },
+       } {
+               api, err := RunFakeArvadosServer(&stub)
+               c.Check(err, IsNil)
+ 
+               defer api.listener.Close()
+ 
+               arv := ArvadosClient{
+                       Scheme:      "http",
+                       ApiServer:   api.url,
+                       ApiToken:    "abc123",
+                       ApiInsecure: true,
+                       Client:      &http.Client{Transport: &http.Transport{}},
+                       Retries:     2}
+ 
+               getback := make(Dict)
+               switch stub.method {
+               case "get":
+                       err = arv.Get("collections", "zzzzz-4zz18-znfnqtbbv4spc3w", nil, &getback)
+               case "create":
+                       err = arv.Create("collections",
+                               Dict{"collection": Dict{"name": "testing"}},
+                               &getback)
+               case "update":
+                       err = arv.Update("collections", "zzzzz-4zz18-znfnqtbbv4spc3w",
+                               Dict{"collection": Dict{"name": "testing"}},
+                               &getback)
+               case "delete":
+                       err = arv.Delete("pipeline_templates", "zzzzz-4zz18-znfnqtbbv4spc3w", nil, &getback)
+               }
+ 
+               switch stub.expected {
+               case 200:
+                       c.Check(err, IsNil)
+                       c.Check(getback["ok"], Equals, "ok")
+               case -1:
+                       c.Check(err, NotNil)
+                       c.Check(err, ErrorMatches, `.*stopped after \d+ redirects`)
+               default:
+                       c.Check(err, NotNil)
+                       c.Check(err, ErrorMatches, fmt.Sprintf("arvados API server error: %d.*", stub.expected))
+                       c.Check(err.(APIServerError).HttpStatusCode, Equals, stub.expected)
+               }
+       }
+ }