Remove duplication of GitHub redirect middleware and recorder in tests (#11341)

I think we should use `httptestutil` more and put little helpers like
these in there.

The GitHubProxyRedirectMiddleware I put in `httpcli` next to the other
middlewares.
This commit is contained in:
Thorsten Ball 2020-06-08 14:25:56 +02:00 committed by GitHub
parent 3fbaed354a
commit f7fa096126
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 56 additions and 88 deletions

View File

@ -760,22 +760,12 @@ func TestSources_ListRepos(t *testing.T) {
func newClientFactory(t testing.TB, name string, mws ...httpcli.Middleware) (*httpcli.Factory, func(testing.TB)) {
cassete := filepath.Join("testdata", "sources", strings.Replace(name, " ", "-", -1))
rec := newRecorder(t, cassete, update(name))
mws = append(mws, githubProxyRedirectMiddleware, gitserverRedirectMiddleware)
mws = append(mws, httpcli.GitHubProxyRedirectMiddleware, gitserverRedirectMiddleware)
mw := httpcli.NewMiddleware(mws...)
return httpcli.NewFactory(mw, httptestutil.NewRecorderOpt(rec)),
func(t testing.TB) { save(t, rec) }
}
func githubProxyRedirectMiddleware(cli httpcli.Doer) httpcli.Doer {
return httpcli.DoerFunc(func(req *http.Request) (*http.Response, error) {
if req.URL.Hostname() == "github-proxy" {
req.URL.Host = "api.github.com"
req.URL.Scheme = "https"
}
return cli.Do(req)
})
}
func gitserverRedirectMiddleware(cli httpcli.Doer) httpcli.Doer {
return httpcli.DoerFunc(func(req *http.Request) (*http.Response, error) {
if req.URL.Hostname() == "gitserver" {

View File

@ -30,6 +30,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/extsvc"
"github.com/sourcegraph/sourcegraph/internal/extsvc/github"
"github.com/sourcegraph/sourcegraph/internal/gitserver"
"github.com/sourcegraph/sourcegraph/internal/httptestutil"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/sourcegraph/sourcegraph/internal/repoupdater"
"github.com/sourcegraph/sourcegraph/internal/repoupdater/protocol"
@ -46,7 +47,7 @@ func TestCampaigns(t *testing.T) {
dbtesting.SetupGlobalTestDB(t)
rcache.SetupForTest(t)
cf, save := newGithubClientFactory(t, "test-campaigns")
cf, save := httptestutil.NewGitHubRecorderFactory(t, *update, "test-campaigns")
defer save()
now := time.Now().UTC().Truncate(time.Microsecond)
@ -618,7 +619,7 @@ func TestChangesetCountsOverTime(t *testing.T) {
dbtesting.SetupGlobalTestDB(t)
rcache.SetupForTest(t)
cf, save := newGithubClientFactory(t, "test-changeset-counts-over-time")
cf, save := httptestutil.NewGitHubRecorderFactory(t, *update, "test-changeset-counts-over-time")
defer save()
userID := insertTestUser(t, dbconn.Global, "changeset-counts-over-time", false)

View File

@ -19,13 +19,11 @@ import (
"testing"
"time"
"github.com/dnaeon/go-vcr/cassette"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/sourcegraph/sourcegraph/cmd/repo-updater/repos"
"github.com/sourcegraph/sourcegraph/internal/campaigns"
"github.com/sourcegraph/sourcegraph/internal/extsvc"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
"github.com/sourcegraph/sourcegraph/internal/httptestutil"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/sourcegraph/sourcegraph/schema"
@ -45,7 +43,7 @@ func testGitHubWebhook(db *sql.DB, userID int32) func(*testing.T) {
truncateTables(t, db, "changeset_jobs", "changeset_events", "changesets")
cf, save := newGithubClientFactory(t, "github-webhooks")
cf, save := httptestutil.NewGitHubRecorderFactory(t, *update, "github-webhooks")
defer save()
secret := "secret"
@ -195,7 +193,7 @@ func testBitbucketWebhook(db *sql.DB, userID int32) func(*testing.T) {
truncateTables(t, db, "changeset_jobs", "changeset_events", "changesets")
cf, save := newGithubClientFactory(t, "bitbucket-webhooks")
cf, save := httptestutil.NewGitHubRecorderFactory(t, *update, "bitbucket-webhooks")
defer save()
secret := "secret"
@ -429,36 +427,3 @@ func marshalJSON(t testing.TB, v interface{}) string {
return string(bs)
}
func newGithubClientFactory(t testing.TB, name string) (*httpcli.Factory, func()) {
t.Helper()
cassete := filepath.Join("testdata/vcr/", strings.Replace(name, " ", "-", -1))
rec, err := httptestutil.NewRecorder(cassete, *update, func(i *cassette.Interaction) error {
return nil
})
if err != nil {
t.Fatal(err)
}
mw := httpcli.NewMiddleware(githubProxyRedirectMiddleware)
hc := httpcli.NewFactory(mw, httptestutil.NewRecorderOpt(rec))
return hc, func() {
if err := rec.Stop(); err != nil {
t.Errorf("failed to update test data: %s", err)
}
}
}
func githubProxyRedirectMiddleware(cli httpcli.Doer) httpcli.Doer {
return httpcli.DoerFunc(func(req *http.Request) (*http.Response, error) {
if req.URL.Hostname() == "github-proxy" {
req.URL.Host = "api.github.com"
req.URL.Scheme = "https"
}
return cli.Do(req)
})
}

View File

@ -7,20 +7,16 @@ import (
"encoding/json"
"flag"
"fmt"
"net/http"
"net/url"
"os"
"path/filepath"
"reflect"
"regexp"
"strconv"
"strings"
"testing"
"github.com/dnaeon/go-vcr/cassette"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
"github.com/sourcegraph/sourcegraph/internal/httpcli"
"github.com/sourcegraph/sourcegraph/internal/httptestutil"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/sourcegraph/sourcegraph/internal/testutil"
@ -444,45 +440,18 @@ func TestClient_GetAuthenticatedUserOrgs(t *testing.T) {
func newClient(t testing.TB, name string) (*Client, func()) {
t.Helper()
cassete := filepath.Join("testdata/vcr/", strings.Replace(name, " ", "-", -1))
rec, err := httptestutil.NewRecorder(cassete, update(name), func(i *cassette.Interaction) error {
return nil
})
if err != nil {
t.Fatal(err)
}
mw := httpcli.NewMiddleware(githubProxyRedirectMiddleware)
hc, err := httpcli.NewFactory(mw, httptestutil.NewRecorderOpt(rec)).Doer()
if err != nil {
t.Fatal(err)
}
cf, save := httptestutil.NewGitHubRecorderFactory(t, update(name), name)
uri, err := url.Parse("https://github.com")
if err != nil {
t.Fatal(err)
}
cli := NewClient(
uri,
os.Getenv("GITHUB_TOKEN"),
hc,
)
return cli, func() {
if err := rec.Stop(); err != nil {
t.Errorf("failed to update test data: %s", err)
}
doer, err := cf.Doer()
if err != nil {
t.Fatal(err)
}
}
func githubProxyRedirectMiddleware(cli httpcli.Doer) httpcli.Doer {
return httpcli.DoerFunc(func(req *http.Request) (*http.Response, error) {
if req.URL.Hostname() == "github-proxy" {
req.URL.Host = "api.github.com"
req.URL.Scheme = "https"
}
return cli.Do(req)
})
cli := NewClient(uri, os.Getenv("GITHUB_TOKEN"), doer)
return cli, save
}

View File

@ -156,6 +156,18 @@ func ContextErrorMiddleware(cli Doer) Doer {
})
}
// GitHubProxyRedirectMiddleware rewrites requests to the "github-proxy" host
// to "https://api.github.com".
func GitHubProxyRedirectMiddleware(cli Doer) Doer {
return DoerFunc(func(req *http.Request) (*http.Response, error) {
if req.URL.Hostname() == "github-proxy" {
req.URL.Host = "api.github.com"
req.URL.Scheme = "https"
}
return cli.Do(req)
})
}
//
// Common Opts
//

View File

@ -2,6 +2,9 @@ package httptestutil
import (
"net/http"
"path/filepath"
"strings"
"testing"
"github.com/dnaeon/go-vcr/cassette"
"github.com/dnaeon/go-vcr/recorder"
@ -48,3 +51,31 @@ func NewRecorderOpt(rec *recorder.Recorder) httpcli.Opt {
return nil
}
}
// NewGitHubRecorderFactory returns a *http.Factory that rewrites HTTP requests to
// github-proxy to github.com and records all HTTP requests in "testdata/vcr/{name}"
// with {name} being the name that's passed in.
// If update is true, the HTTP requests are recorded, otherwise they're
// replayed from the recorded cassete.
func NewGitHubRecorderFactory(t testing.TB, update bool, name string) (*httpcli.Factory, func()) {
t.Helper()
cassete := filepath.Join("testdata/vcr/", strings.Replace(name, " ", "-", -1))
rec, err := NewRecorder(cassete, update, func(i *cassette.Interaction) error {
return nil
})
if err != nil {
t.Fatal(err)
}
mw := httpcli.NewMiddleware(httpcli.GitHubProxyRedirectMiddleware)
hc := httpcli.NewFactory(mw, NewRecorderOpt(rec))
return hc, func() {
if err := rec.Stop(); err != nil {
t.Errorf("failed to update test data: %s", err)
}
}
}