Skip to content

Commit

Permalink
Remove regex from readiness tests (#383)
Browse files Browse the repository at this point in the history
The refactored tests assert that when a service's readiness endpoint fails, it returns an error including the URL used to call the endpoint. Currently, the URL is matched against a regex, which is not ideal. This PR replaces the regex with a normal string, which is much easier to read.
  • Loading branch information
Anton-Kalpakchiev authored Nov 20, 2024
1 parent 84d958c commit 059a132
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 41 deletions.
53 changes: 26 additions & 27 deletions build-index/tagserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import (
"io/ioutil"
"net/http"
"net/url"
"regexp"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -153,34 +153,34 @@ func TestHealth(t *testing.T) {

func TestCheckReadiness(t *testing.T) {
for _, tc := range []struct {
name string
mockStatErr error
mockOriginErr error
expectedErrMsgPattern string
name string
statErr error
originErr error
wantErr string
}{
{
name: "success",
mockStatErr: nil,
mockOriginErr: nil,
expectedErrMsgPattern: "",
name: "success",
statErr: nil,
originErr: nil,
wantErr: "",
},
{
name: "failure, 503 (only Stat fails)",
mockStatErr: errors.New("backend storage error"),
mockOriginErr: nil,
expectedErrMsgPattern: fmt.Sprintf(`build index not ready: GET http://127\.0\.0\.1:\d+/readiness 503: not ready to serve traffic: backend for namespace 'foo-bar/\*' not ready: backend storage error`),
name: "failure, 503 (only Stat fails)",
statErr: errors.New("backend storage error"),
originErr: nil,
wantErr: "build index not ready: GET http://{address}/readiness 503: not ready to serve traffic: backend for namespace 'foo-bar/*' not ready: backend storage error",
},
{
name: "failure, 503 (only origin fails)",
mockStatErr: nil,
mockOriginErr: errors.New("origin error"),
expectedErrMsgPattern: fmt.Sprintf(`build index not ready: GET http://127\.0\.0\.1:\d+/readiness 503: not ready to serve traffic: origin error`),
name: "failure, 503 (only origin fails)",
statErr: nil,
originErr: errors.New("origin error"),
wantErr: "build index not ready: GET http://{address}/readiness 503: not ready to serve traffic: origin error",
},
{
name: "failure, 503 (both fail)",
mockStatErr: errors.New("backend storage error"),
mockOriginErr: errors.New("origin error"),
expectedErrMsgPattern: fmt.Sprintf(`build index not ready: GET http://127\.0\.0\.1:\d+/readiness 503: not ready to serve traffic: backend for namespace 'foo-bar/\*' not ready: backend storage error`),
name: "failure, 503 (both fail)",
statErr: errors.New("backend storage error"),
originErr: errors.New("origin error"),
wantErr: "build index not ready: GET http://{address}/readiness 503: not ready to serve traffic: backend for namespace 'foo-bar/*' not ready: backend storage error",
},
} {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -197,18 +197,17 @@ func TestCheckReadiness(t *testing.T) {
require.NoError(mocks.backends.Register("foo-bar/*", backendClient, true))

mockStat := &core.BlobInfo{}
if tc.mockStatErr != nil {
if tc.statErr != nil {
mockStat = nil
}
backendClient.EXPECT().Stat(backend.ReadinessCheckNamespace, backend.ReadinessCheckName).Return(mockStat, tc.mockStatErr)
mocks.originClient.EXPECT().CheckReadiness().Return(tc.mockOriginErr).AnyTimes()
backendClient.EXPECT().Stat(backend.ReadinessCheckNamespace, backend.ReadinessCheckName).Return(mockStat, tc.statErr)
mocks.originClient.EXPECT().CheckReadiness().Return(tc.originErr).AnyTimes()

err := client.CheckReadiness()
if tc.expectedErrMsgPattern == "" {
if tc.wantErr == "" {
require.Nil(err)
} else {
r, _ := regexp.Compile(tc.expectedErrMsgPattern)
require.True(r.MatchString(err.Error()))
require.EqualError(err, strings.ReplaceAll(tc.wantErr, "{address}", addr))
}
})
}
Expand Down
27 changes: 13 additions & 14 deletions tracker/trackerserver/announce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ package trackerserver
import (
"errors"
"fmt"
"regexp"
"strings"
"testing"
"time"

Expand All @@ -36,19 +36,19 @@ func newAnnounceClient(pctx core.PeerContext, addr string) announceclient.Client

func TestCheckReadiness(t *testing.T) {
for _, tc := range []struct {
name string
mockOriginErr error
expectedErrMsgPattern string
name string
originErr error
wantErr string
}{
{
name: "success",
mockOriginErr: nil,
expectedErrMsgPattern: "",
name: "success",
originErr: nil,
wantErr: "",
},
{
name: "failure, 503 (origin fails)",
mockOriginErr: errors.New("origin error"),
expectedErrMsgPattern: fmt.Sprintf(`tracker not ready: GET http://127\.0\.0\.1:\d+/readiness 503: not ready to serve traffic: origin error`),
name: "failure, 503 (origin fails)",
originErr: errors.New("origin error"),
wantErr: "tracker not ready: GET http://{address}/readiness 503: not ready to serve traffic: origin error",
},
} {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -60,17 +60,16 @@ func TestCheckReadiness(t *testing.T) {
addr, stop := testutil.StartServer(mocks.handler())
defer stop()

mocks.originCluster.EXPECT().CheckReadiness().Return(tc.mockOriginErr)
mocks.originCluster.EXPECT().CheckReadiness().Return(tc.originErr)

pctx := core.PeerContextFixture()
client := newAnnounceClient(pctx, addr)

err := client.CheckReadiness()
if tc.expectedErrMsgPattern == "" {
if tc.wantErr == "" {
require.Nil(err)
} else {
r, _ := regexp.Compile(tc.expectedErrMsgPattern)
require.True(r.MatchString(err.Error()))
require.EqualError(err, strings.ReplaceAll(tc.wantErr, "{address}", addr))
}
})
}
Expand Down

0 comments on commit 059a132

Please sign in to comment.