storage: remove dependency to sql/catalog/bootstrap

[Xiang-Gu]

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 #82172 if interested).

Release note: None

[cockroach-teamcity]

This change is 

[ajwerner]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Xiang-Gu)

---

pkg/storage/BUILD.bazel line 192 at r1 (raw file):

    disallowed_list = [
        "//pkg/sql/catalog/bootstrap",
    ],

I don't think this does what you think it does. I think this only ensure that the library itself does not depend on bootstrap. Sadly, I don't think there's a way right now to represent the test dependencies.

Code quote:

disallowed_imports_test(
    src = "storage",
    disallowed_list = [
        "//pkg/sql/catalog/bootstrap",
    ],

---

pkg/storage/helpers_test.go line 11 at r1 (raw file):

// licenses/APL.txt.

package storage_test

consider calling this file external_helpers_test.go. I think of helpers_test.go as the conventional file for forwarding unexported references from the current packages to tests defined in the _test package. This is going the other way.

---

pkg/storage/mvcc_test.go line 5018 at r1 (raw file):

}

var TestingUserTableDataMin func() roachpb.Key

note that this is injected via external_helpers_test.go, same for the other var

[Xiang-Gu]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

---

pkg/storage/BUILD.bazel line 192 at r1 (raw file):

Previously, ajwerner wrote…

I don't think this does what you think it does. I think this only ensure that the library itself does not depend on bootstrap. Sadly, I don't think there's a way right now to represent the test dependencies.

So, this rule will ignore files that end with *_test.go, so even if one *_test.go file (packaged storage) imports /pkg/sql/catalog/bootstrap, it won't catch and disallow it, right?

---

pkg/storage/mvcc_test.go line 5018 at r1 (raw file):

Previously, ajwerner wrote…

note that this is injected via external_helpers_test.go, same for the other var

done

---

pkg/storage/helpers_test.go line 11 at r1 (raw file):

Previously, ajwerner wrote…

consider calling this file external_helpers_test.go. I think of helpers_test.go as the conventional file for forwarding unexported references from the current packages to tests defined in the _test package. This is going the other way.

done

[ajwerner]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Xiang-Gu)

[Xiang-Gu]

bors r+

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build succeeded:

• GitHub CI (Cockroach)