From 21b2f72b467f9ef54b322e5928ecfe0e62c0081a Mon Sep 17 00:00:00 2001 From: Luke Bjerring Date: Mon, 12 Nov 2018 09:32:11 -0500 Subject: [PATCH] Avoid circular deps --- api/query/autocomplete_medium_test.go | 2 +- api/query/query_test.go | 5 +- api/query/search_medium_test.go | 4 +- api/receiver/receive_results_test.go | 16 +- shared/models_test.go | 5 +- shared/request_caching_test.go | 23 ++- shared/sharedtest/util.go | 8 + shared/storage_mock.go | 213 -------------------------- shared/storage_test.go | 21 +-- shared/util.go | 7 - 10 files changed, 54 insertions(+), 250 deletions(-) delete mode 100644 shared/storage_mock.go diff --git a/api/query/autocomplete_medium_test.go b/api/query/autocomplete_medium_test.go index 714ed9cc1a8..2a892793444 100644 --- a/api/query/autocomplete_medium_test.go +++ b/api/query/autocomplete_medium_test.go @@ -66,7 +66,7 @@ func TestAutocompleteHandler(t *testing.T) { defer mockCtrl.Finish() // TODO(markdittmer): Should this be hitting GCS instead? - store := shared.NewMockReadable(mockCtrl) + store := sharedtest.NewMockReadable(mockCtrl) rs := []*sharedtest.MockReadCloser{ sharedtest.NewMockReadCloser(t, summaryBytes[0]), sharedtest.NewMockReadCloser(t, summaryBytes[1]), diff --git a/api/query/query_test.go b/api/query/query_test.go index 6dbf489b822..9540bb661af 100644 --- a/api/query/query_test.go +++ b/api/query/query_test.go @@ -13,6 +13,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/web-platform-tests/wpt.fyi/shared" + "github.com/web-platform-tests/wpt.fyi/shared/sharedtest" ) func TestGetMemcacheKey(t *testing.T) { @@ -44,7 +45,7 @@ func TestLoadSummaries_success(t *testing.T) { getMemcacheKey(testRuns[1]), } - cachedStore := shared.NewMockCachedStore(mockCtrl) + cachedStore := sharedtest.NewMockCachedStore(mockCtrl) sh := unstructuredSearchHandler{queryHandler{dataSource: cachedStore}} summaryBytes := [][]byte{ []byte(`{"/a/b/c":[1,2]}`), @@ -94,7 +95,7 @@ func TestLoadSummaries_fail(t *testing.T) { getMemcacheKey(testRuns[1]), } - cachedStore := shared.NewMockCachedStore(mockCtrl) + cachedStore := sharedtest.NewMockCachedStore(mockCtrl) sh := unstructuredSearchHandler{queryHandler{dataSource: cachedStore}} summaryBytes := [][]byte{ []byte(`{"/a/b/c":[1,2]}`), diff --git a/api/query/search_medium_test.go b/api/query/search_medium_test.go index 6a524a9b6b8..9242ef5f6ac 100644 --- a/api/query/search_medium_test.go +++ b/api/query/search_medium_test.go @@ -67,7 +67,7 @@ func TestUnstructuredSearchHandler(t *testing.T) { defer mockCtrl.Finish() // TODO(markdittmer): Should this be hitting GCS instead? - store := shared.NewMockReadable(mockCtrl) + store := sharedtest.NewMockReadable(mockCtrl) rs := []*sharedtest.MockReadCloser{ sharedtest.NewMockReadCloser(t, summaryBytes[0]), sharedtest.NewMockReadCloser(t, summaryBytes[1]), @@ -187,7 +187,7 @@ func TestStructuredSearchHandler_equivalentToUnstructured(t *testing.T) { defer mockCtrl.Finish() // TODO(markdittmer): Should this be hitting GCS instead? - store := shared.NewMockReadable(mockCtrl) + store := sharedtest.NewMockReadable(mockCtrl) rs := []*sharedtest.MockReadCloser{ sharedtest.NewMockReadCloser(t, summaryBytes[0]), sharedtest.NewMockReadCloser(t, summaryBytes[1]), diff --git a/api/receiver/receive_results_test.go b/api/receiver/receive_results_test.go index 2150274d158..d21597e6335 100644 --- a/api/receiver/receive_results_test.go +++ b/api/receiver/receive_results_test.go @@ -18,7 +18,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" - "github.com/web-platform-tests/wpt.fyi/shared" + "github.com/web-platform-tests/wpt.fyi/shared/sharedtest" "google.golang.org/appengine/taskqueue" ) @@ -92,7 +92,7 @@ func TestHandleResultsUpload_success(t *testing.T) { mockAE := NewMockAppEngineAPI(mockCtrl) f := &os.File{} task := &taskqueue.Task{Name: "task"} - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().IsAdmin().Return(false), mockAE.EXPECT().authenticateUploader("blade-runner", "123").Return(true), @@ -130,7 +130,7 @@ func TestHandleResultsUpload_extra_params(t *testing.T) { "os_version": "", } task := &taskqueue.Task{Name: "task"} - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().IsAdmin().Return(false), mockAE.EXPECT().authenticateUploader("blade-runner", "123").Return(true), @@ -171,7 +171,7 @@ func TestHandleURLPayload_single(t *testing.T) { } mockAE := NewMockAppEngineAPI(mockCtrl) - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().fetchWithTimeout("http://wpt.fyi/test.json.gz", DownloadTimeout).Return(f, nil), mockAE.EXPECT().uploadToGCS(matchRegex(`^/wptd-results-buffer/blade-runner/.*\.json$`), f, true).Return(nil), @@ -189,7 +189,7 @@ func TestHandleURLPayload_multiple(t *testing.T) { urls := []string{"http://wpt.fyi/foo.json.gz", "http://wpt.fyi/bar.json.gz"} mockAE := NewMockAppEngineAPI(mockCtrl) - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().fetchWithTimeout(urls[0], DownloadTimeout).Return(f, nil), mockAE.EXPECT().uploadToGCS(matchRegex(`^/wptd-results-buffer/blade-runner/.*/0\.json$`), f, true).Return(nil), @@ -211,7 +211,7 @@ func TestHandleURLPayload_retry_fetching(t *testing.T) { errTimeout := fmt.Errorf("server timed out") mockAE := NewMockAppEngineAPI(mockCtrl) - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().fetchWithTimeout("http://wpt.fyi/test.json.gz", DownloadTimeout).Return(nil, errTimeout), mockAE.EXPECT().fetchWithTimeout("http://wpt.fyi/test.json.gz", DownloadTimeout).Return(nil, errTimeout), @@ -230,7 +230,7 @@ func TestHandleURLPayload_fail_fetching(t *testing.T) { errTimeout := fmt.Errorf("server timed out") mockAE := NewMockAppEngineAPI(mockCtrl) - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().fetchWithTimeout("http://wpt.fyi/test.json.gz", DownloadTimeout).Return(nil, errTimeout), mockAE.EXPECT().fetchWithTimeout("http://wpt.fyi/test.json.gz", DownloadTimeout).Return(nil, errTimeout), @@ -250,7 +250,7 @@ func TestHandleURLPayload_fail_uploading(t *testing.T) { errGCS := fmt.Errorf("failed to upload to GCS") mockAE := NewMockAppEngineAPI(mockCtrl) - mockAE.EXPECT().Context().Return(shared.NewTestContext()).AnyTimes() + mockAE.EXPECT().Context().Return(sharedtest.NewTestContext()).AnyTimes() gomock.InOrder( mockAE.EXPECT().fetchWithTimeout("http://wpt.fyi/test.json.gz", DownloadTimeout).Return(f, nil), mockAE.EXPECT().uploadToGCS(matchRegex(`^/wptd-results-buffer/blade-runner/.*\.json$`), f, true).Return(errGCS), diff --git a/shared/models_test.go b/shared/models_test.go index 6fdfa122dfb..eceea25c97f 100644 --- a/shared/models_test.go +++ b/shared/models_test.go @@ -1,17 +1,18 @@ // +build medium -package shared +package shared_test import ( "testing" "github.com/stretchr/testify/assert" + "github.com/web-platform-tests/wpt.fyi/shared" "github.com/web-platform-tests/wpt.fyi/shared/sharedtest" "google.golang.org/appengine/datastore" ) func TestTestRunIDs_LoadTestRuns(t *testing.T) { - testRuns := make(TestRuns, 2) + testRuns := make(shared.TestRuns, 2) testRuns[0].BrowserName = "chrome" testRuns[0].BrowserVersion = "63.0" testRuns[0].OSName = "linux" diff --git a/shared/request_caching_test.go b/shared/request_caching_test.go index 8d770637f7f..c6b9e7ca092 100644 --- a/shared/request_caching_test.go +++ b/shared/request_caching_test.go @@ -4,7 +4,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -package shared +package shared_test import ( "context" @@ -16,6 +16,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/web-platform-tests/wpt.fyi/shared" "github.com/web-platform-tests/wpt.fyi/shared/sharedtest" ) @@ -36,9 +37,15 @@ func (okHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { func TestNoCaching404(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - cache := NewMockReadWritable(mockCtrl) + cache := sharedtest.NewMockReadWritable(mockCtrl) cache.EXPECT().NewReadCloser("/some/url").Return(ioutil.NopCloser(failReader{}), nil) - h := NewCachingHandler(context.Background(), http.NotFoundHandler(), cache, AlwaysCachable, URLAsCacheKey, CacheStatusOK) + h := shared.NewCachingHandler( + context.Background(), + http.NotFoundHandler(), + cache, + shared.AlwaysCachable, + shared.URLAsCacheKey, + shared.CacheStatusOK) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/some/url", nil) h.ServeHTTP(w, r) @@ -48,11 +55,17 @@ func TestNoCaching404(t *testing.T) { func TestCaching200(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() - cache := NewMockReadWritable(mockCtrl) + cache := sharedtest.NewMockReadWritable(mockCtrl) cache.EXPECT().NewReadCloser("/some/url").Return(ioutil.NopCloser(failReader{}), nil) wc := sharedtest.NewMockWriteCloser(t) cache.EXPECT().NewWriteCloser("/some/url").Return(wc, nil) - h := NewCachingHandler(context.Background(), okHandler{}, cache, AlwaysCachable, URLAsCacheKey, CacheStatusOK) + h := shared.NewCachingHandler( + context.Background(), + okHandler{}, + cache, + shared.AlwaysCachable, + shared.URLAsCacheKey, + shared.CacheStatusOK) w := httptest.NewRecorder() r := httptest.NewRequest("GET", "/some/url", nil) h.ServeHTTP(w, r) diff --git a/shared/sharedtest/util.go b/shared/sharedtest/util.go index 654793d5904..6bde42213fc 100644 --- a/shared/sharedtest/util.go +++ b/shared/sharedtest/util.go @@ -7,6 +7,7 @@ package sharedtest import ( "context" + "github.com/web-platform-tests/wpt.fyi/shared" "google.golang.org/appengine" "google.golang.org/appengine/aetest" ) @@ -39,3 +40,10 @@ func NewAEContext(stronglyConsistentDatastore bool) (context.Context, func(), er inst.Close() }, nil } + +// NewTestContext creates a new context.Context for small tests. +func NewTestContext() context.Context { + ctx := context.Background() + ctx = context.WithValue(ctx, shared.DefaultLoggerCtxKey(), shared.NewNilLogger()) + return ctx +} diff --git a/shared/storage_mock.go b/shared/storage_mock.go deleted file mode 100644 index be3bb24b5c3..00000000000 --- a/shared/storage_mock.go +++ /dev/null @@ -1,213 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: shared/storage.go - -// Package shared is a generated GoMock package. -package shared - -import ( - gomock "github.com/golang/mock/gomock" - io "io" - reflect "reflect" -) - -// MockReadable is a mock of Readable interface -type MockReadable struct { - ctrl *gomock.Controller - recorder *MockReadableMockRecorder -} - -// MockReadableMockRecorder is the mock recorder for MockReadable -type MockReadableMockRecorder struct { - mock *MockReadable -} - -// NewMockReadable creates a new mock instance -func NewMockReadable(ctrl *gomock.Controller) *MockReadable { - mock := &MockReadable{ctrl: ctrl} - mock.recorder = &MockReadableMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockReadable) EXPECT() *MockReadableMockRecorder { - return m.recorder -} - -// NewReadCloser mocks base method -func (m *MockReadable) NewReadCloser(arg0 interface{}) (io.ReadCloser, error) { - ret := m.ctrl.Call(m, "NewReadCloser", arg0) - ret0, _ := ret[0].(io.ReadCloser) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NewReadCloser indicates an expected call of NewReadCloser -func (mr *MockReadableMockRecorder) NewReadCloser(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewReadCloser", reflect.TypeOf((*MockReadable)(nil).NewReadCloser), arg0) -} - -// MockReadWritable is a mock of ReadWritable interface -type MockReadWritable struct { - ctrl *gomock.Controller - recorder *MockReadWritableMockRecorder -} - -// MockReadWritableMockRecorder is the mock recorder for MockReadWritable -type MockReadWritableMockRecorder struct { - mock *MockReadWritable -} - -// NewMockReadWritable creates a new mock instance -func NewMockReadWritable(ctrl *gomock.Controller) *MockReadWritable { - mock := &MockReadWritable{ctrl: ctrl} - mock.recorder = &MockReadWritableMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockReadWritable) EXPECT() *MockReadWritableMockRecorder { - return m.recorder -} - -// NewReadCloser mocks base method -func (m *MockReadWritable) NewReadCloser(arg0 interface{}) (io.ReadCloser, error) { - ret := m.ctrl.Call(m, "NewReadCloser", arg0) - ret0, _ := ret[0].(io.ReadCloser) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NewReadCloser indicates an expected call of NewReadCloser -func (mr *MockReadWritableMockRecorder) NewReadCloser(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewReadCloser", reflect.TypeOf((*MockReadWritable)(nil).NewReadCloser), arg0) -} - -// NewWriteCloser mocks base method -func (m *MockReadWritable) NewWriteCloser(arg0 interface{}) (io.WriteCloser, error) { - ret := m.ctrl.Call(m, "NewWriteCloser", arg0) - ret0, _ := ret[0].(io.WriteCloser) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NewWriteCloser indicates an expected call of NewWriteCloser -func (mr *MockReadWritableMockRecorder) NewWriteCloser(arg0 interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewWriteCloser", reflect.TypeOf((*MockReadWritable)(nil).NewWriteCloser), arg0) -} - -// MockCachedStore is a mock of CachedStore interface -type MockCachedStore struct { - ctrl *gomock.Controller - recorder *MockCachedStoreMockRecorder -} - -// MockCachedStoreMockRecorder is the mock recorder for MockCachedStore -type MockCachedStoreMockRecorder struct { - mock *MockCachedStore -} - -// NewMockCachedStore creates a new mock instance -func NewMockCachedStore(ctrl *gomock.Controller) *MockCachedStore { - mock := &MockCachedStore{ctrl: ctrl} - mock.recorder = &MockCachedStoreMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockCachedStore) EXPECT() *MockCachedStoreMockRecorder { - return m.recorder -} - -// Get mocks base method -func (m *MockCachedStore) Get(cacheID, storeID, value interface{}) error { - ret := m.ctrl.Call(m, "Get", cacheID, storeID, value) - ret0, _ := ret[0].(error) - return ret0 -} - -// Get indicates an expected call of Get -func (mr *MockCachedStoreMockRecorder) Get(cacheID, storeID, value interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockCachedStore)(nil).Get), cacheID, storeID, value) -} - -// MockObjectStore is a mock of ObjectStore interface -type MockObjectStore struct { - ctrl *gomock.Controller - recorder *MockObjectStoreMockRecorder -} - -// MockObjectStoreMockRecorder is the mock recorder for MockObjectStore -type MockObjectStoreMockRecorder struct { - mock *MockObjectStore -} - -// NewMockObjectStore creates a new mock instance -func NewMockObjectStore(ctrl *gomock.Controller) *MockObjectStore { - mock := &MockObjectStore{ctrl: ctrl} - mock.recorder = &MockObjectStoreMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockObjectStore) EXPECT() *MockObjectStoreMockRecorder { - return m.recorder -} - -// Get mocks base method -func (m *MockObjectStore) Get(id, value interface{}) error { - ret := m.ctrl.Call(m, "Get", id, value) - ret0, _ := ret[0].(error) - return ret0 -} - -// Get indicates an expected call of Get -func (mr *MockObjectStoreMockRecorder) Get(id, value interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockObjectStore)(nil).Get), id, value) -} - -// MockObjectCache is a mock of ObjectCache interface -type MockObjectCache struct { - ctrl *gomock.Controller - recorder *MockObjectCacheMockRecorder -} - -// MockObjectCacheMockRecorder is the mock recorder for MockObjectCache -type MockObjectCacheMockRecorder struct { - mock *MockObjectCache -} - -// NewMockObjectCache creates a new mock instance -func NewMockObjectCache(ctrl *gomock.Controller) *MockObjectCache { - mock := &MockObjectCache{ctrl: ctrl} - mock.recorder = &MockObjectCacheMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockObjectCache) EXPECT() *MockObjectCacheMockRecorder { - return m.recorder -} - -// Get mocks base method -func (m *MockObjectCache) Get(id, value interface{}) error { - ret := m.ctrl.Call(m, "Get", id, value) - ret0, _ := ret[0].(error) - return ret0 -} - -// Get indicates an expected call of Get -func (mr *MockObjectCacheMockRecorder) Get(id, value interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockObjectCache)(nil).Get), id, value) -} - -// Put mocks base method -func (m *MockObjectCache) Put(id, value interface{}) error { - ret := m.ctrl.Call(m, "Put", id, value) - ret0, _ := ret[0].(error) - return ret0 -} - -// Put indicates an expected call of Put -func (mr *MockObjectCacheMockRecorder) Put(id, value interface{}) *gomock.Call { - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Put", reflect.TypeOf((*MockObjectCache)(nil).Put), id, value) -} diff --git a/shared/storage_test.go b/shared/storage_test.go index 5c66c134b11..43ddd548d50 100644 --- a/shared/storage_test.go +++ b/shared/storage_test.go @@ -4,7 +4,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -package shared +package shared_test import ( "errors" @@ -12,6 +12,7 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" + "github.com/web-platform-tests/wpt.fyi/shared" "github.com/web-platform-tests/wpt.fyi/shared/sharedtest" "google.golang.org/appengine/memcache" ) @@ -22,9 +23,9 @@ func TestGet_cacheHit(t *testing.T) { var cacheID, storeID interface{} - cache := NewMockReadWritable(mockCtrl) - store := NewMockReadable(mockCtrl) - cs := NewByteCachedStore(NewTestContext(), cache, store) + cache := sharedtest.NewMockReadWritable(mockCtrl) + store := sharedtest.NewMockReadable(mockCtrl) + cs := shared.NewByteCachedStore(sharedtest.NewTestContext(), cache, store) data := []byte("{}") cr := sharedtest.NewMockReadCloser(t, data) @@ -42,9 +43,9 @@ func TestGet_cacheMiss(t *testing.T) { var cacheID, storeID interface{} - cache := NewMockReadWritable(mockCtrl) - store := NewMockReadable(mockCtrl) - cs := NewByteCachedStore(NewTestContext(), cache, store) + cache := sharedtest.NewMockReadWritable(mockCtrl) + store := sharedtest.NewMockReadable(mockCtrl) + cs := shared.NewByteCachedStore(sharedtest.NewTestContext(), cache, store) data := []byte("{}") cw := sharedtest.NewMockWriteCloser(t) @@ -66,9 +67,9 @@ func TestGet_missing(t *testing.T) { var cacheID, storeID interface{} - cache := NewMockReadWritable(mockCtrl) - store := NewMockReadable(mockCtrl) - cs := NewByteCachedStore(NewTestContext(), cache, store) + cache := sharedtest.NewMockReadWritable(mockCtrl) + store := sharedtest.NewMockReadable(mockCtrl) + cs := shared.NewByteCachedStore(sharedtest.NewTestContext(), cache, store) errMissing := errors.New("Failed to fetch from store") cache.EXPECT().NewReadCloser(&cacheID).Return(nil, memcache.ErrCacheMiss) diff --git a/shared/util.go b/shared/util.go index 0bbed20d77a..98b80b06d14 100644 --- a/shared/util.go +++ b/shared/util.go @@ -195,13 +195,6 @@ func NewRequestContext(r *http.Request) context.Context { return ctx } -// NewTestContext creates a new context.Context for small tests. -func NewTestContext() context.Context { - ctx := context.Background() - ctx = context.WithValue(ctx, DefaultLoggerCtxKey(), NewNilLogger()) - return ctx -} - // NewSetFromStringSlice is a helper for the inability to cast []string to []interface{} func NewSetFromStringSlice(items []string) mapset.Set { if items == nil {