Skip to content

Commit cdb8a43

Browse files
committed
tests, logictest, floatcmp: refactor comparison test util functions
This commit moves some float comparison test util functions from logictest into the floatcmp package. It also moves a query result comparison function from the tlp file to query_comparison_util in the tests package. This commit also marks roachtests as testonly targets. Epic: none Release note: None
1 parent 9149e0e commit cdb8a43

File tree

9 files changed

+159
-146
lines changed

9 files changed

+159
-146
lines changed

pkg/cmd/roachtest/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
22

33
go_library(
44
name = "roachtest_lib",
5+
testonly = 1,
56
srcs = [
67
"cluster.go",
78
"github.go",
@@ -62,13 +63,15 @@ go_library(
6263

6364
go_binary(
6465
name = "roachtest",
66+
testonly = 1,
6567
embed = [":roachtest_lib"],
6668
visibility = ["//visibility:public"],
6769
)
6870

6971
go_test(
7072
name = "roachtest_test",
7173
size = "small",
74+
testonly = 1,
7275
srcs = [
7376
"cluster_test.go",
7477
"github_test.go",

pkg/cmd/roachtest/tests/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
33

44
go_library(
55
name = "tests",
6+
testonly = 1,
67
srcs = [
78
"acceptance.go",
89
"activerecord.go",
@@ -224,6 +225,7 @@ go_library(
224225
"//pkg/storage",
225226
"//pkg/storage/enginepb",
226227
"//pkg/testutils",
228+
"//pkg/testutils/floatcmp",
227229
"//pkg/testutils/jobutils",
228230
"//pkg/testutils/release",
229231
"//pkg/testutils/sqlutils",
@@ -298,6 +300,7 @@ go_test(
298300
"//pkg/roachprod/logger",
299301
"//pkg/roachprod/prometheus",
300302
"//pkg/testutils/skip",
303+
"//pkg/util/leaktest",
301304
"//pkg/util/version",
302305
"@com_github_golang_mock//gomock",
303306
"@com_github_google_go_github//github",

pkg/cmd/roachtest/tests/query_comparison_util.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
"math/rand"
1919
"os"
2020
"path/filepath"
21+
"runtime"
22+
"sort"
2123
"strings"
2224
"time"
2325

@@ -28,10 +30,12 @@ import (
2830
"github.com/cockroachdb/cockroach/pkg/internal/sqlsmith"
2931
"github.com/cockroachdb/cockroach/pkg/internal/workloadreplay"
3032
"github.com/cockroachdb/cockroach/pkg/roachprod/install"
33+
"github.com/cockroachdb/cockroach/pkg/testutils/floatcmp"
3134
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
3235
"github.com/cockroachdb/cockroach/pkg/util/randutil"
3336
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
3437
"github.com/cockroachdb/errors"
38+
"github.com/google/go-cmp/cmp"
3539
)
3640

3741
const (
@@ -504,3 +508,18 @@ func (h *queryComparisonHelper) logVerboseOutput() {
504508
func (h *queryComparisonHelper) makeError(err error, msg string) error {
505509
return errors.Wrapf(err, "%s. %d statements run", msg, h.stmtNo)
506510
}
511+
512+
// unsortedMatricesDiff sorts and compares rows of data.
513+
func unsortedMatricesDiff(rowMatrix1, rowMatrix2 [][]string) string {
514+
var rows1 []string
515+
for _, row := range rowMatrix1 {
516+
rows1 = append(rows1, strings.Join(row[:], ","))
517+
}
518+
var rows2 []string
519+
for _, row := range rowMatrix2 {
520+
rows2 = append(rows2, strings.Join(row[:], ","))
521+
}
522+
sort.Strings(rows1)
523+
sort.Strings(rows2)
524+
return cmp.Diff(rows1, rows2)
525+
}

pkg/cmd/roachtest/tests/tlp.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"fmt"
1717
"os"
1818
"path/filepath"
19-
"sort"
2019
"strings"
2120
"time"
2221

@@ -29,7 +28,6 @@ import (
2928
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
3029
"github.com/cockroachdb/cockroach/pkg/util/randutil"
3130
"github.com/cockroachdb/errors"
32-
"github.com/google/go-cmp/cmp"
3331
)
3432

3533
const statementTimeout = time.Minute
@@ -271,20 +269,6 @@ func runTLPQuery(conn *gosql.DB, smither *sqlsmith.Smither, logStmt func(string)
271269
})
272270
}
273271

274-
func unsortedMatricesDiff(rowMatrix1, rowMatrix2 [][]string) string {
275-
var rows1 []string
276-
for _, row := range rowMatrix1 {
277-
rows1 = append(rows1, strings.Join(row[:], ","))
278-
}
279-
var rows2 []string
280-
for _, row := range rowMatrix2 {
281-
rows2 = append(rows2, strings.Join(row[:], ","))
282-
}
283-
sort.Strings(rows1)
284-
sort.Strings(rows2)
285-
return cmp.Diff(rows1, rows2)
286-
}
287-
288272
func runWithTimeout(f func() error) error {
289273
done := make(chan error, 1)
290274
go func() {

pkg/sql/logictest/logic.go

Lines changed: 2 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
gosql "database/sql"
1919
"flag"
2020
"fmt"
21-
"math"
2221
"math/rand"
2322
"net"
2423
"net/url"
@@ -3676,9 +3675,9 @@ func (t *logicTest) finishExecQuery(query logicQuery, rows *gosql.Rows, err erro
36763675
// ('R') coltypes are approximately equal to take into account
36773676
// platform differences in floating point calculations.
36783677
if runtime.GOARCH == "s390x" && (colT == 'F' || colT == 'R') {
3679-
resultMatches, err = floatsMatchApprox(expected, actual)
3678+
resultMatches, err = floatcmp.FloatsMatchApprox(expected, actual)
36803679
} else if colT == 'F' {
3681-
resultMatches, err = floatsMatch(expected, actual)
3680+
resultMatches, err = floatcmp.FloatsMatch(expected, actual)
36823681
}
36833682
if err != nil {
36843683
return errors.CombineErrors(makeError(), err)
@@ -3746,93 +3745,6 @@ func (t *logicTest) finishExecQuery(query logicQuery, rows *gosql.Rows, err erro
37463745
return nil
37473746
}
37483747

3749-
// parseExpectedAndActualFloats converts the strings expectedString and
3750-
// actualString to float64 values.
3751-
func parseExpectedAndActualFloats(expectedString, actualString string) (float64, float64, error) {
3752-
expected, err := strconv.ParseFloat(expectedString, 64 /* bitSize */)
3753-
if err != nil {
3754-
return 0, 0, errors.Wrap(err, "when parsing expected")
3755-
}
3756-
actual, err := strconv.ParseFloat(actualString, 64 /* bitSize */)
3757-
if err != nil {
3758-
return 0, 0, errors.Wrap(err, "when parsing actual")
3759-
}
3760-
return expected, actual, nil
3761-
}
3762-
3763-
// floatsMatchApprox returns whether two floating point represented as
3764-
// strings are equal within a tolerance.
3765-
func floatsMatchApprox(expectedString, actualString string) (bool, error) {
3766-
expected, actual, err := parseExpectedAndActualFloats(expectedString, actualString)
3767-
if err != nil {
3768-
return false, err
3769-
}
3770-
return floatcmp.EqualApprox(expected, actual, floatcmp.CloseFraction, floatcmp.CloseMargin), nil
3771-
}
3772-
3773-
// floatsMatch returns whether two floating point numbers represented as
3774-
// strings have matching 15 significant decimal digits (this is the precision
3775-
// that Postgres supports for 'double precision' type).
3776-
func floatsMatch(expectedString, actualString string) (bool, error) {
3777-
expected, actual, err := parseExpectedAndActualFloats(expectedString, actualString)
3778-
if err != nil {
3779-
return false, err
3780-
}
3781-
// Check special values - NaN, +Inf, -Inf, 0.
3782-
if math.IsNaN(expected) || math.IsNaN(actual) {
3783-
return math.IsNaN(expected) == math.IsNaN(actual), nil
3784-
}
3785-
if math.IsInf(expected, 0 /* sign */) || math.IsInf(actual, 0 /* sign */) {
3786-
bothNegativeInf := math.IsInf(expected, -1 /* sign */) == math.IsInf(actual, -1 /* sign */)
3787-
bothPositiveInf := math.IsInf(expected, 1 /* sign */) == math.IsInf(actual, 1 /* sign */)
3788-
return bothNegativeInf || bothPositiveInf, nil
3789-
}
3790-
if expected == 0 || actual == 0 {
3791-
return expected == actual, nil
3792-
}
3793-
// Check that the numbers have the same sign.
3794-
if expected*actual < 0 {
3795-
return false, nil
3796-
}
3797-
expected = math.Abs(expected)
3798-
actual = math.Abs(actual)
3799-
// Check that 15 significant digits match. We do so by normalizing the
3800-
// numbers and then checking one digit at a time.
3801-
//
3802-
// normalize converts f to base * 10**power representation where base is in
3803-
// [1.0, 10.0) range.
3804-
normalize := func(f float64) (base float64, power int) {
3805-
for f >= 10 {
3806-
f = f / 10
3807-
power++
3808-
}
3809-
for f < 1 {
3810-
f *= 10
3811-
power--
3812-
}
3813-
return f, power
3814-
}
3815-
var expPower, actPower int
3816-
expected, expPower = normalize(expected)
3817-
actual, actPower = normalize(actual)
3818-
if expPower != actPower {
3819-
return false, nil
3820-
}
3821-
// TODO(yuzefovich): investigate why we can't always guarantee deterministic
3822-
// 15 significant digits and switch back from 14 to 15 digits comparison
3823-
// here. See #56446 for more details.
3824-
for i := 0; i < 14; i++ {
3825-
expDigit := int(expected)
3826-
actDigit := int(actual)
3827-
if expDigit != actDigit {
3828-
return false, nil
3829-
}
3830-
expected -= (expected - float64(expDigit)) * 10
3831-
actual -= (actual - float64(actDigit)) * 10
3832-
}
3833-
return true, nil
3834-
}
3835-
38363748
func (t *logicTest) formatValues(vals []string, valsPerLine int) []string {
38373749
var buf bytes.Buffer
38383750
tw := tabwriter.NewWriter(&buf, 2, 1, 2, ' ', 0)

pkg/sql/logictest/main_test.go

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
_ "github.com/cockroachdb/cockroach/pkg/sql/tests"
2121
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
2222
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
23-
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
2423
"github.com/cockroachdb/cockroach/pkg/util/randutil"
2524
)
2625

@@ -33,41 +32,3 @@ func TestMain(m *testing.M) {
3332
serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
3433
os.Exit(m.Run())
3534
}
36-
37-
// TestFloatsMatch is a unit test for floatsMatch() and floatsMatchApprox()
38-
// functions.
39-
func TestFloatsMatch(t *testing.T) {
40-
defer leaktest.AfterTest(t)()
41-
for _, tc := range []struct {
42-
f1, f2 string
43-
match bool
44-
}{
45-
{f1: "NaN", f2: "+Inf", match: false},
46-
{f1: "+Inf", f2: "+Inf", match: true},
47-
{f1: "NaN", f2: "NaN", match: true},
48-
{f1: "+Inf", f2: "-Inf", match: false},
49-
{f1: "-0.0", f2: "0.0", match: true},
50-
{f1: "0.0", f2: "NaN", match: false},
51-
{f1: "123.45", f2: "12.345", match: false},
52-
{f1: "0.1234567890123456", f2: "0.1234567890123455", match: true},
53-
{f1: "0.1234567890123456", f2: "0.1234567890123457", match: true},
54-
{f1: "-0.1234567890123456", f2: "0.1234567890123456", match: false},
55-
{f1: "-0.1234567890123456", f2: "-0.1234567890123455", match: true},
56-
} {
57-
match, err := floatsMatch(tc.f1, tc.f2)
58-
if err != nil {
59-
t.Fatal(err)
60-
}
61-
if match != tc.match {
62-
t.Fatalf("floatsMatch: wrong result on %v", tc)
63-
}
64-
65-
match, err = floatsMatchApprox(tc.f1, tc.f2)
66-
if err != nil {
67-
t.Fatal(err)
68-
}
69-
if match != tc.match {
70-
t.Fatalf("floatsMatchApprox: wrong result on %v", tc)
71-
}
72-
}
73-
}

pkg/testutils/floatcmp/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ go_library(
77
importpath = "github.com/cockroachdb/cockroach/pkg/testutils/floatcmp",
88
visibility = ["//visibility:public"],
99
deps = [
10+
"@com_github_cockroachdb_errors//:errors",
1011
"@com_github_google_go_cmp//cmp",
1112
"@com_github_google_go_cmp//cmp/cmpopts",
1213
],
@@ -18,4 +19,5 @@ go_test(
1819
srcs = ["floatcmp_test.go"],
1920
args = ["-test.timeout=55s"],
2021
embed = [":floatcmp"],
22+
deps = ["//pkg/util/leaktest"],
2123
)

0 commit comments

Comments
 (0)