Skip to content

Commit

Permalink
internal/frontend: remove experiment middleware from newTestServer
Browse files Browse the repository at this point in the history
No experiments are actually set in the callers to newTestServer or
testServer.

For golang/go#61399

Change-Id: Iae58a670cc17356bd24634e6ff17ee9f70ca5f69
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/514520
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
kokoro-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
matloob committed Aug 4, 2023
1 parent 30cc364 commit 051a825
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 39 deletions.
16 changes: 3 additions & 13 deletions internal/frontend/frontend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/google/safehtml/template"
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/middleware"
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/proxy/proxytest"
"golang.org/x/pkgsite/internal/queue"
Expand Down Expand Up @@ -45,13 +44,13 @@ type testPackage struct {
docs []*internal.Documentation
}

func newTestServer(t *testing.T, proxyModules []*proxytest.Module, cacher Cacher, experimentNames ...string) (*Server, http.Handler, func()) {
func newTestServer(t *testing.T, proxyModules []*proxytest.Module, cacher Cacher) (*Server, http.Handler, func()) {
t.Helper()
proxyClient, teardown := proxytest.SetupTestClient(t, proxyModules)
sourceClient := source.NewClient(sourceTimeout)
ctx := context.Background()

q := queue.NewInMemory(ctx, 1, experimentNames,
q := queue.NewInMemory(ctx, 1, nil,
func(ctx context.Context, mpath, version string) (int, error) {
return FetchAndUpdateState(ctx, mpath, version, proxyClient, sourceClient, testDB)
})
Expand All @@ -73,16 +72,7 @@ func newTestServer(t *testing.T, proxyModules []*proxytest.Module, cacher Cacher
mux := http.NewServeMux()
s.Install(mux.Handle, cacher, nil)

var exps []*internal.Experiment
for _, n := range experimentNames {
exps = append(exps, &internal.Experiment{Name: n, Rollout: 100})
}
exp, err := middleware.NewExperimenter(ctx, time.Hour, func(context.Context) ([]*internal.Experiment, error) { return exps, nil }, nil)
if err != nil {
t.Fatal(err)
}
mw := middleware.Experiment(exp)
return s, mw(mux), func() {
return s, mux, func() {
teardown()
postgres.ResetTestDB(testDB, t)
}
Expand Down
29 changes: 3 additions & 26 deletions internal/frontend/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/cookie"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/experiment"
"golang.org/x/pkgsite/internal/middleware"
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/testing/htmlcheck"
Expand Down Expand Up @@ -62,8 +61,6 @@ type serverTestCase struct {
wantLocation string
// if non-nil, call the checker on the HTML root node
want htmlcheck.Checker
// list of experiments that must be enabled for this test to run
requiredExperiments *experiment.Set
}

// Units with this prefix will be marked as excluded.
Expand Down Expand Up @@ -1102,7 +1099,6 @@ func TestServer(t *testing.T) {
for _, test := range []struct {
name string
testCasesFunc func() []serverTestCase
experiments []string
}{
{
name: "no experiments",
Expand All @@ -1114,32 +1110,23 @@ func TestServer(t *testing.T) {
},
} {
t.Run(test.name, func(t *testing.T) {
testServer(t, test.testCasesFunc(), test.experiments...)
testServer(t, test.testCasesFunc())
})
}
}

func testServer(t *testing.T, testCases []serverTestCase, experimentNames ...string) {
func testServer(t *testing.T, testCases []serverTestCase) {
ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
defer cancel()
defer postgres.ResetTestDB(testDB, t)

// Experiments need to be set in the context, for DB work, and as a
// middleware, for request handling.
ctx = experiment.NewContext(ctx, experimentNames...)
insertTestModules(ctx, t, testModules)
if err := testDB.InsertExcludedPrefix(ctx, excludedModulePath, "testuser", "testreason"); err != nil {
t.Fatal(err)
}
_, handler, _ := newTestServer(t, nil, nil, experimentNames...)

experimentsSet := experiment.NewSet(experimentNames...)
_, handler, _ := newTestServer(t, nil, nil)

for _, test := range testCases {
if !isSubset(test.requiredExperiments, experimentsSet) {
continue
}

t.Run(test.name, func(t *testing.T) { // remove initial '/' for name
w := httptest.NewRecorder()
handler.ServeHTTP(w, httptest.NewRequest("GET", test.urlPath, nil))
Expand Down Expand Up @@ -1170,16 +1157,6 @@ func testServer(t *testing.T, testCases []serverTestCase, experimentNames ...str
}
}

func isSubset(subset, set *experiment.Set) bool {
for _, e := range subset.Active() {
if !set.IsActive(e) {
return false
}
}

return true
}

func TestServerErrors(t *testing.T) {
_, handler, _ := newTestServer(t, nil, nil)
for _, test := range []struct {
Expand Down

0 comments on commit 051a825

Please sign in to comment.