change flag, comments

Makefile

@@ -41,7 +41,7 @@ VISIBILITY_DB ?= temporal_visibility
 # Always use "protolegacy" tag to allow disabling utf-8 validation on proto messages
 # during proto library transition.
 ALL_BUILD_TAGS := protolegacy,$(BUILD_TAG)
-ALL_TEST_TAGS := $(ALL_BUILD_TAGS),testhooks,$(TEST_TAG)
+ALL_TEST_TAGS := $(ALL_BUILD_TAGS),test_dep,$(TEST_TAG)
 BUILD_TAG_FLAG := -tags $(ALL_BUILD_TAGS)
 TEST_TAG_FLAG := -tags $(ALL_TEST_TAGS)
 

common/testing/testhooks/noop_impl.go

@@ -20,7 +20,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-//go:build !testhooks
+//go:build !test_dep
 
 package testhooks
 
@@ -31,9 +31,13 @@ var Module = fx.Options(
 )
 
 type (
+	// TestHooks (in production mode) is an empty struct just so the build works.
+	// See TestHooks in test_impl.go.
 	TestHooks struct{}
 )
 
+// Get gets the value of a test hook. In production mode it always returns the zero value and
+// false, which hopefully the compiler will inline and remove the hook as dead code.
 func Get[T any](_ TestHooks, key string) (T, bool) {
 	var zero T
 	return zero, false

common/testing/testhooks/test_impl.go

@@ -20,7 +20,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-//go:build testhooks
+//go:build test_dep
 
 package testhooks
 
@@ -35,18 +35,22 @@ var Module = fx.Options(
 )
 
 type (
+	// TestHooks holds a registry of active test hooks. It should be obtained through fx and
+	// used with Get and Set.
 	TestHooks interface {
 		// private accessors; access must go through package-level Get/Set
 		get(string) (any, bool)
 		set(string, any)
 		del(string)
 	}
 
+	// testHooksImpl is an implementation of TestHooks.
 	testHooksImpl struct {
 		m sync.Map
 	}
 )
 
+// Get gets the value of a test hook from the registry.
 func Get[T any](th TestHooks, key string) (T, bool) {
 	if val, ok := th.get(key); ok {
 		// this is only used in test so we want to panic on type mismatch:
@@ -56,18 +60,22 @@ func Get[T any](th TestHooks, key string) (T, bool) {
 	return zero, false
 }
 
+// Set sets a test hook to a value and returns a cleanup function to unset it.
+// Calls to Set and the cleanup functions should form a stack.
 func Set[T any](th TestHooks, key string, val T) func() {
 	th.set(key, val)
 	return func() { th.del(key) }
 }
 
+// NewTestHooksImpl returns a new instance of a test hook registry. This is provided and used
+// in the main "resource" module as a default, but in functional tests, it's overridden by an
+// explicitly constructed instance.
 func NewTestHooksImpl() TestHooks {
 	return &testHooksImpl{}
 }
 
 func (th *testHooksImpl) get(key string) (any, bool) {
-	val, ok := th.m.Load(key)
-	return val, ok
+	return th.m.Load(key)
 }
 
 func (th *testHooksImpl) set(key string, val any) {