util,build: use crdb_test tag instead of metamorphic

This commit switches the builds to become "metamorphic" based on the
recently introduced `crdb_test` tag (which is appended to all test
targets) instead of `metamorphic`. The corresponding teamcity config is
removed because we will get the metamorphic runs with the regular test
runs now.

Note that since some logic tests cannot be run when some constants
initialized to non-default value, in order to not lose the test coverage
for those tests, even if `crdb_test` build is specified, with 20%
probability the build will not be metamorphic.

Additionally, this commit switches the logic of
`ConstantWithMetamorphic*` methods to also choose the default value (in
25% cases). The reasoning for this change is that we want to make sure
to run the tests with the default values relatively often.

To summarize, the current behavior is:
- `crdb_test` build tag is specified (all test targets):
  - with 20% probability the build is not metamorphic, so all magic
constants get the default value.
  - with 80% probability the build is metamorphic. Every value being
metamorphized has 75% of being initialized to non-default value.
- `crdb_test` build is not specified:
  - the build is not metamorphic, so all magic constants get the
produciton value.

Another minor change is a fix to
`row.TestingSetDatumRowConverterBatchSize` to properly reset a value to
its old one in the returned closure.

`TestMysqldumpDataReader` is currently skipped when the metamorphic
build occurs pending further investigation.

Release note: None

pkg/bench/ddl_analysis/validate_benchmark_data_test.go

@@ -63,6 +63,7 @@ func TestBenchmarkExpectation(t *testing.T) {
 	skip.UnderStress(t)
 	skip.UnderRace(t)
 	skip.UnderShort(t)
+	skip.UnderMetamorphic(t)
 
 	expecations := readExpectationsFile(t)
 

pkg/ccl/importccl/read_import_mysql_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/row"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
@@ -41,6 +42,10 @@ func TestMysqldumpDataReader(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
+	// This test currently blocks indefinitely if
+	// row.kvDatumRowConverterBatchSize is set to 1 or 2 (#57955).
+	skip.UnderMetamorphic(t)
+
 	files := getMysqldumpTestdata(t)
 
 	ctx := context.Background()

pkg/sql/logictest/logic.go

@@ -1593,7 +1593,7 @@ func processConfigs(t *testing.T, path string, defaults configSet, configNames [
 	}
 
 	var configs configSet
-	if util.MetamorphicBuild {
+	if util.IsMetamorphicBuild() {
 		for c := range blocklist {
 			if c == "metamorphic" {
 				// We have a metamorphic build and the file has !metamorphic

pkg/sql/row/row_converter.go

@@ -230,12 +230,13 @@ var kvDatumRowConverterBatchSize = util.ConstantWithMetamorphicTestValue(
 	1,    /* metamorphicValue */
 )
 
-// TestingSetDatumRowConverterBatchSize sets kvDatumRowConverterBatchSize and returns function to
-// reset this setting back to its old value.
+// TestingSetDatumRowConverterBatchSize sets kvDatumRowConverterBatchSize and
+// returns function to reset this setting back to its old value.
 func TestingSetDatumRowConverterBatchSize(newSize int) func() {
+	oldSize := kvDatumRowConverterBatchSize
 	kvDatumRowConverterBatchSize = newSize
 	return func() {
-		kvDatumRowConverterBatchSize = 5000
+		kvDatumRowConverterBatchSize = oldSize
 	}
 }
 

pkg/testutils/skip/skip.go

@@ -94,7 +94,7 @@ func UnderStressRace(t SkippableTest, args ...interface{}) {
 // run with the metamorphic build tag.
 func UnderMetamorphic(t SkippableTest, args ...interface{}) {
 	t.Helper()
-	if util.MetamorphicBuild {
+	if util.IsMetamorphicBuild() {
 		t.Skip(append([]interface{}{"disabled under metamorphic"}, args...))
 	}
 }

pkg/util/BUILD.bazel

@@ -4,12 +4,12 @@ go_library(
     name = "util",
     srcs = [
         "constants.go",
+        "crdb_test_off.go",
         "every_n.go",
         "fast_int_map.go",
         "fast_int_set.go",
         "fast_int_set_str.go",
         "hash.go",
-        "metamorphic_off.go",
         "nocopy.go",
         "pluralize.go",
         "race_off.go",

pkg/util/constants.go

@@ -18,10 +18,28 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 )
 
-// ConstantWithMetamorphicTestValue should be used to initialize "magic constants" that
-// should be varied during test scenarios to check for bugs at boundary
-// conditions. When built with the test_constants build tag, the test value
-// will be used. In all other cases, the production value will be used.
+// IsMetamorphicBuild returns whether this build is metamorphic. By build being
+// "metamorphic" we mean that some magic constants in the codebase might get
+// initialized to non-default value.
+// A build will become metamorphic with metamorphicBuildProbability probability
+// if 'crdb_test' build flag is specified (this is the case for all test
+// targets).
+func IsMetamorphicBuild() bool {
+	return metamorphicBuild
+}
+
+var metamorphicBuild bool
+
+const (
+	metamorphicBuildProbability = 0.8
+	metamorphicValueProbability = 0.75
+)
+
+// ConstantWithMetamorphicTestValue should be used to initialize "magic
+// constants" that should be varied during test scenarios to check for bugs at
+// boundary conditions. When metamorphicBuild is true, the test value will be
+// used with metamorphicValueProbability probability. In all other cases, the
+// production ("default") value will be used.
 // The constant must be a "metamorphic variable": changing it cannot affect the
 // output of any SQL DMLs. It can only affect the way in which the data is
 // retrieved or processed, because otherwise the main test corpus would fail if
@@ -40,36 +58,41 @@ import (
 //
 // var batchSize = util.ConstantWithMetamorphicTestValue(64, 1)
 //
-// This will give your code a batch size of 1 in the test_constants build
+// This will often give your code a batch size of 1 in the crdb_test build
 // configuration, increasing the amount of exercise the edge conditions get.
 func ConstantWithMetamorphicTestValue(defaultValue, metamorphicValue int) int {
-	if MetamorphicBuild {
-		logMetamorphicValue(metamorphicValue)
-		return metamorphicValue
+	if metamorphicBuild {
+		if rng.Float64() < metamorphicValueProbability {
+			logMetamorphicValue(metamorphicValue)
+			return metamorphicValue
+		}
 	}
 	return defaultValue
 }
 
-// rng is initialized to a rand.Rand if MetamorphicBuild is enabled.
+// rng is initialized to a rand.Rand if crdbTestBuild is enabled.
 var rng *rand.Rand
 
 func init() {
-	if MetamorphicBuild {
+	if crdbTestBuild {
 		rng, _ = randutil.NewPseudoRand()
+		metamorphicBuild = rng.Float64() < metamorphicBuildProbability
 	}
 }
 
 // ConstantWithMetamorphicTestRange is like ConstantWithMetamorphicTestValue
 // except instead of returning a single metamorphic test value, it returns a
 // random test value in a range.
 func ConstantWithMetamorphicTestRange(defaultValue, min, max int) int {
-	if MetamorphicBuild {
-		ret := min
-		if max > min {
-			ret = int(rng.Int31())%(max-min) + min
+	if metamorphicBuild {
+		if rng.Float64() < metamorphicValueProbability {
+			ret := min
+			if max > min {
+				ret = int(rng.Int31())%(max-min) + min
+			}
+			logMetamorphicValue(ret)
+			return ret
 		}
-		logMetamorphicValue(ret)
-		return ret
 	}
 	return defaultValue
 }

pkg/util/crdb_test_off.go

@@ -0,0 +1,20 @@
+// Copyright 2020 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.
+
+// +build !crdb_test
+
+package util
+
+// crdbTestBuild is a flag that is set to true if the binary was compiled
+// with the 'crdb_test' build tag (which is the case for all test targets). This
+// flag can be used to enable expensive checks, test randomizations, or other
+// metamorphic-style perturbations that will not affect test results but will
+// exercise different parts of the code.
+const crdbTestBuild = false

pkg/util/crdb_test_on.go

@@ -0,0 +1,20 @@
+// Copyright 2020 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.
+
+// +build crdb_test
+
+package util
+
+// crdbTestBuild is a flag that is set to true if the binary was compiled
+// with the 'crdb_test' build tag (which is the case for all test targets). This
+// flag can be used to enable expensive checks, test randomizations, or other
+// metamorphic-style perturbations that will not affect test results but will
+// exercise different parts of the code.
+const crdbTestBuild = true