From 0685eb53951a81e8553f1165e353018da5cff13d Mon Sep 17 00:00:00 2001 From: Chris Marchbanks Date: Thu, 8 Aug 2019 19:35:39 -0600 Subject: [PATCH] Refactor testutil.NewStorage into a new package This avoids a circular dependency between the testutil and storage packages. Signed-off-by: Chris Marchbanks --- promql/bench_test.go | 4 ++-- promql/functions_test.go | 3 ++- promql/test.go | 5 +++-- rules/manager_test.go | 9 +++++---- rules/recording_test.go | 3 ++- scrape/scrape_test.go | 13 +++++++------ storage/tsdb/tsdb_test.go | 3 ++- util/{testutil => teststorage}/storage.go | 7 ++++--- web/api/v1/api_test.go | 3 ++- 9 files changed, 29 insertions(+), 21 deletions(-) rename util/{testutil => teststorage}/storage.go (90%) diff --git a/promql/bench_test.go b/promql/bench_test.go index 1f8fa7947..4358b05c5 100644 --- a/promql/bench_test.go +++ b/promql/bench_test.go @@ -22,11 +22,11 @@ import ( "time" "github.com/prometheus/prometheus/pkg/labels" - "github.com/prometheus/prometheus/util/testutil" + "github.com/prometheus/prometheus/util/teststorage" ) func BenchmarkRangeQuery(b *testing.B) { - storage := testutil.NewStorage(b) + storage := teststorage.New(b) defer storage.Close() opts := EngineOpts{ Logger: nil, diff --git a/promql/functions_test.go b/promql/functions_test.go index d7134b3d7..bd0ced3f4 100644 --- a/promql/functions_test.go +++ b/promql/functions_test.go @@ -20,6 +20,7 @@ import ( "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/pkg/timestamp" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -27,7 +28,7 @@ func TestDeriv(t *testing.T) { // https://github.com/prometheus/prometheus/issues/2674#issuecomment-315439393 // This requires more precision than the usual test system offers, // so we test it by hand. - storage := testutil.NewStorage(t) + storage := teststorage.New(t) defer storage.Close() opts := EngineOpts{ Logger: nil, diff --git a/promql/test.go b/promql/test.go index 3632de4a0..292203100 100644 --- a/promql/test.go +++ b/promql/test.go @@ -28,6 +28,7 @@ import ( "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -509,7 +510,7 @@ func (t *Test) clear() { if t.cancelCtx != nil { t.cancelCtx() } - t.storage = testutil.NewStorage(t) + t.storage = teststorage.New(t) opts := EngineOpts{ Logger: nil, @@ -623,7 +624,7 @@ func (ll *LazyLoader) clear() { if ll.cancelCtx != nil { ll.cancelCtx() } - ll.storage = testutil.NewStorage(ll) + ll.storage = teststorage.New(ll) opts := EngineOpts{ Logger: nil, diff --git a/rules/manager_test.go b/rules/manager_test.go index bd0c5363e..69ce91490 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -28,6 +28,7 @@ import ( "github.com/prometheus/prometheus/pkg/value" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -492,7 +493,7 @@ func TestForStateRestore(t *testing.T) { } func TestStaleness(t *testing.T) { - storage := testutil.NewStorage(t) + storage := teststorage.New(t) defer storage.Close() engineOpts := promql.EngineOpts{ Logger: nil, @@ -633,7 +634,7 @@ func TestCopyState(t *testing.T) { } func TestDeletedRuleMarkedStale(t *testing.T) { - storage := testutil.NewStorage(t) + storage := teststorage.New(t) defer storage.Close() oldGroup := &Group{ rules: []Rule{ @@ -679,7 +680,7 @@ func TestUpdate(t *testing.T) { expected := map[string]labels.Labels{ "test": labels.FromStrings("name", "value"), } - storage := testutil.NewStorage(t) + storage := teststorage.New(t) defer storage.Close() opts := promql.EngineOpts{ Logger: nil, @@ -718,7 +719,7 @@ func TestUpdate(t *testing.T) { } func TestNotify(t *testing.T) { - storage := testutil.NewStorage(t) + storage := teststorage.New(t) defer storage.Close() engineOpts := promql.EngineOpts{ Logger: nil, diff --git a/rules/recording_test.go b/rules/recording_test.go index 97133f9e4..8ca607a14 100644 --- a/rules/recording_test.go +++ b/rules/recording_test.go @@ -21,11 +21,12 @@ import ( "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/pkg/timestamp" "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) func TestRuleEval(t *testing.T) { - storage := testutil.NewStorage(t) + storage := teststorage.New(t) defer storage.Close() opts := promql.EngineOpts{ diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 1b4ff0c32..3c3ea20fa 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -42,6 +42,7 @@ import ( "github.com/prometheus/prometheus/pkg/timestamp" "github.com/prometheus/prometheus/pkg/value" "github.com/prometheus/prometheus/storage" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -663,7 +664,7 @@ test_metric 1 func TestScrapeLoopSeriesAdded(t *testing.T) { // Need a full storage for correct Add/AddFast semantics. - s := testutil.NewStorage(t) + s := teststorage.New(t) defer s.Close() app, err := s.Appender() @@ -818,7 +819,7 @@ func TestScrapeLoopRunCreatesStaleMarkersOnParseFailure(t *testing.T) { } func TestScrapeLoopCache(t *testing.T) { - s := testutil.NewStorage(t) + s := teststorage.New(t) defer s.Close() sapp, err := s.Appender() @@ -897,7 +898,7 @@ func TestScrapeLoopCache(t *testing.T) { } func TestScrapeLoopCacheMemoryExhaustionProtection(t *testing.T) { - s := testutil.NewStorage(t) + s := teststorage.New(t) defer s.Close() sapp, err := s.Appender() @@ -1106,7 +1107,7 @@ func TestScrapeLoop_ChangingMetricString(t *testing.T) { // This is a regression test for the scrape loop cache not properly maintaining // IDs when the string representation of a metric changes across a scrape. Thus // we use a real storage appender here. - s := testutil.NewStorage(t) + s := teststorage.New(t) defer s.Close() app, err := s.Appender() @@ -1546,7 +1547,7 @@ func (ts *testScraper) scrape(ctx context.Context, w io.Writer) (string, error) } func TestScrapeLoop_RespectTimestamps(t *testing.T) { - s := testutil.NewStorage(t) + s := teststorage.New(t) defer s.Close() app, err := s.Appender() @@ -1583,7 +1584,7 @@ func TestScrapeLoop_RespectTimestamps(t *testing.T) { } func TestScrapeLoop_DiscardTimestamps(t *testing.T) { - s := testutil.NewStorage(t) + s := teststorage.New(t) defer s.Close() app, err := s.Appender() diff --git a/storage/tsdb/tsdb_test.go b/storage/tsdb/tsdb_test.go index 85a8e43a9..c7813a0ea 100644 --- a/storage/tsdb/tsdb_test.go +++ b/storage/tsdb/tsdb_test.go @@ -20,11 +20,12 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/storage/tsdb" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) func TestMetrics(t *testing.T) { - db := testutil.NewStorage(t) + db := teststorage.New(t) defer db.Close() metrics := &dto.Metric{} diff --git a/util/testutil/storage.go b/util/teststorage/storage.go similarity index 90% rename from util/testutil/storage.go rename to util/teststorage/storage.go index 246d6190d..9b1fcb88b 100644 --- a/util/testutil/storage.go +++ b/util/teststorage/storage.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package testutil +package teststorage import ( "io/ioutil" @@ -21,11 +21,12 @@ import ( "github.com/prometheus/common/model" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/storage/tsdb" + "github.com/prometheus/prometheus/util/testutil" ) -// NewStorage returns a new storage for testing purposes +// New returns a new storage for testing purposes // that removes all associated files on closing. -func NewStorage(t T) storage.Storage { +func New(t testutil.T) storage.Storage { dir, err := ioutil.TempDir("", "test_storage") if err != nil { t.Fatalf("Opening test dir failed: %s", err) diff --git a/web/api/v1/api_test.go b/web/api/v1/api_test.go index 0499b40f4..05defe96f 100644 --- a/web/api/v1/api_test.go +++ b/web/api/v1/api_test.go @@ -50,6 +50,7 @@ import ( "github.com/prometheus/prometheus/scrape" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/storage/remote" + "github.com/prometheus/prometheus/util/teststorage" "github.com/prometheus/prometheus/util/testutil" ) @@ -165,7 +166,7 @@ func (m rulesRetrieverMock) AlertingRules() []*rules.AlertingRule { func (m rulesRetrieverMock) RuleGroups() []*rules.Group { var ar rulesRetrieverMock arules := ar.AlertingRules() - storage := testutil.NewStorage(m.testing) + storage := teststorage.New(m.testing) defer storage.Close() engineOpts := promql.EngineOpts{