From 051a825d5ae1981874f79fb4d5bb32c1ae6696ba Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Mon, 31 Jul 2023 18:52:52 -0400 Subject: [PATCH] internal/frontend: remove experiment middleware from newTestServer 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 Run-TryBot: Michael Matloob kokoro-CI: kokoro TryBot-Result: Gopher Robot --- internal/frontend/frontend_test.go | 16 +++------------- internal/frontend/server_test.go | 29 +++-------------------------- 2 files changed, 6 insertions(+), 39 deletions(-) diff --git a/internal/frontend/frontend_test.go b/internal/frontend/frontend_test.go index ff35894f4..7a7066a75 100644 --- a/internal/frontend/frontend_test.go +++ b/internal/frontend/frontend_test.go @@ -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" @@ -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) }) @@ -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) } diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go index 0bd94a160..d5926ddb7 100644 --- a/internal/frontend/server_test.go +++ b/internal/frontend/server_test.go @@ -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" @@ -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. @@ -1102,7 +1099,6 @@ func TestServer(t *testing.T) { for _, test := range []struct { name string testCasesFunc func() []serverTestCase - experiments []string }{ { name: "no experiments", @@ -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)) @@ -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 {