Skip to content

Commit

Permalink
storage: remove dependency to sql/catalog/bootstrap
Browse files Browse the repository at this point in the history
Previously, tests in `pkg/storage` depended on `sql/catalog/bootstrap`.
This was inadequate/weird because `storage` is a much lower layer in the
architectural stack. It will also help prevent dependency cycles in
other PRs when we introduce depencies (see cockroachdb#82172 if interested).

Release note: None
  • Loading branch information
Xiang-Gu committed Jul 1, 2022
1 parent d324df1 commit 0e2f329
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 4 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ ALL_TESTS = [
"//pkg/storage/enginepb:enginepb_test",
"//pkg/storage/fs:fs_test",
"//pkg/storage/metamorphic:metamorphic_test",
"//pkg/storage:storage_disallowed_imports_test",
"//pkg/storage:storage_test",
"//pkg/testutils/floatcmp:floatcmp_test",
"//pkg/testutils/keysutils:keysutils_test",
Expand Down
9 changes: 9 additions & 0 deletions pkg/storage/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
load("//build:STRINGER.bzl", "stringer")
load("//pkg/testutils/buildutil:buildutil.bzl", "disallowed_imports_test")

go_library(
name = "storage",
Expand Down Expand Up @@ -104,6 +105,7 @@ go_test(
"disk_map_test.go",
"engine_key_test.go",
"engine_test.go",
"helpers_test.go",
"intent_interleaving_iter_test.go",
"intent_reader_writer_test.go",
"main_test.go",
Expand Down Expand Up @@ -182,3 +184,10 @@ stringer(
src = "resource_limiter.go",
typ = "ResourceLimitReached",
)

disallowed_imports_test(
src = "storage",
disallowed_list = [
"//pkg/sql/catalog/bootstrap",
],
)
21 changes: 21 additions & 0 deletions pkg/storage/helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package storage_test

import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
"github.com/cockroachdb/cockroach/pkg/storage"
)

func init() {
storage.TestingUserDescID = bootstrap.TestingUserDescID
storage.TestingUserTableDataMin = bootstrap.TestingUserTableDataMin
}
11 changes: 7 additions & 4 deletions pkg/storage/mvcc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/bootstrap"
"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
Expand Down Expand Up @@ -4374,13 +4373,15 @@ func TestFindSplitKey(t *testing.T) {
}
}

var TestingUserDescID func(offset uint32) uint32

// TestFindValidSplitKeys verifies split keys are located such that
// they avoid splits through invalid key ranges.
func TestFindValidSplitKeys(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

userID := bootstrap.TestingUserDescID(0)
userID := TestingUserDescID(0)
// Manually creates rows corresponding to the schema:
// CREATE TABLE t (id1 STRING, id2 STRING, ... PRIMARY KEY (id1, id2, ...))
addTablePrefix := func(prefix roachpb.Key, id uint32, rowVals ...string) roachpb.Key {
Expand Down Expand Up @@ -4574,7 +4575,7 @@ func TestFindValidSplitKeys(t *testing.T) {
addColFam(tablePrefix(userID, "b"), 1),
addColFam(tablePrefix(userID, "c"), 1),
},
rangeStart: keys.SystemSQLCodec.TablePrefix(bootstrap.TestingUserDescID(0)),
rangeStart: keys.SystemSQLCodec.TablePrefix(TestingUserDescID(0)),
expSplit: tablePrefix(userID, "b"),
expError: false,
},
Expand Down Expand Up @@ -5014,6 +5015,8 @@ func (it *seekLTTrackingIterator) SeekLT(k MVCCKey) {
it.MVCCIterator.SeekLT(k)
}

var TestingUserTableDataMin func() roachpb.Key

// TestMVCCGarbageCollectUsesSeekLTAppropriately ensures that the garbage
// collection only utilizes SeekLT if there are enough undeleted versions.
func TestMVCCGarbageCollectUsesSeekLTAppropriately(t *testing.T) {
Expand All @@ -5037,7 +5040,7 @@ func TestMVCCGarbageCollectUsesSeekLTAppropriately(t *testing.T) {
batch := engine.NewBatch()
defer batch.Close()
it := batch.NewMVCCIterator(MVCCKeyAndIntentsIterKind, IterOptions{
UpperBound: bootstrap.TestingUserTableDataMin(),
UpperBound: TestingUserTableDataMin(),
LowerBound: keys.MaxKey,
})
defer it.Close()
Expand Down

0 comments on commit 0e2f329

Please sign in to comment.