Skip to content

Commit 5898b5d

Browse files
craig[bot]tbg
andcommitted
Merge #150782
150782: asim: move plots to `testdata/generated` r=tbg a=tbg Stacked on #150774. ---- This PR moves the plot to a subdirectory not walked by the datadriven tests. I have work in progress that automatically generates all possible plots for each test. It's better to keep them apart at that point since there's a fair number of files. Additionally, I'd like to make "reports" (like an HTML file) comparing different allocator configurations, and so we're going to have even more structure that should be outside the realm of the datadriven file walker. Epic: CRDB-25222 Co-authored-by: Tobias Grieger <[email protected]>
2 parents afef2d2 + ee849b3 commit 5898b5d

File tree

210 files changed

+171
-154
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

210 files changed

+171
-154
lines changed

pkg/kv/kvserver/asim/tests/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ go_test(
3333
srcs = [
3434
"datadriven_simulation_test.go",
3535
"helpers_test.go",
36+
"plotting_test.go",
3637
"rand_test.go",
3738
],
3839
data = glob(["testdata/**"]),

pkg/kv/kvserver/asim/tests/datadriven_simulation_test.go

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"context"
1010
"fmt"
1111
"hash/fnv"
12-
"io"
1312
"math/rand"
1413
"os"
1514
"path/filepath"
@@ -37,10 +36,6 @@ import (
3736
"github.com/cockroachdb/datadriven"
3837
"github.com/cockroachdb/logtags"
3938
"github.com/stretchr/testify/require"
40-
"gonum.org/v1/plot"
41-
"gonum.org/v1/plot/plotter"
42-
"gonum.org/v1/plot/plotutil"
43-
"gonum.org/v1/plot/vg"
4439
)
4540

4641
var runAsimTests = envutil.EnvOrDefaultBool("COCKROACH_RUN_ASIM_TESTS", false)
@@ -197,9 +192,6 @@ func TestDataDriven(t *testing.T) {
197192
leakTestAfter()
198193
})
199194
datadriven.Walk(t, dir, func(t *testing.T, path string) {
200-
if filepath.Ext(path) != ".txt" {
201-
return
202-
}
203195
ctx := logtags.AddTag(context.Background(), "name", filepath.Base(path))
204196
// The inline comment below is required for TestLint/TestTParallel.
205197
// We use t.Cleanup to work around the issue this lint is trying to prevent.
@@ -213,7 +205,6 @@ func TestDataDriven(t *testing.T) {
213205
assertions := []assertion.SimulationAssertion{}
214206
var stateStrAcrossSamples []string
215207
var runs []history.History
216-
plotNum := 0
217208
datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
218209
defer func() {
219210
require.Empty(t, d.CmdArgs, "leftover arguments for %s", d.Cmd)
@@ -411,7 +402,7 @@ func TestDataDriven(t *testing.T) {
411402

412403
return ""
413404
case "eval":
414-
t.Logf("running eval for %s", path)
405+
t.Logf("running eval for %s", filepath.Base(path))
415406
samples := 1
416407
seed := rand.Int63()
417408
duration := 30 * time.Minute
@@ -592,52 +583,27 @@ func TestDataDriven(t *testing.T) {
592583
buf.WriteString(s)
593584
buf.WriteString("\n")
594585

595-
// The plot's filename is determined by the name of the test file and
596-
// the plot counter within the file.
597-
plotNum++
598-
plotDir := filepath.Dir(path)
599-
fileName := fmt.Sprintf(
600-
"%s_%d_%s.png",
601-
strings.TrimSuffix(filepath.Base(path), filepath.Ext(path)),
602-
plotNum,
603-
stat,
604-
)
605-
filePath := filepath.Join(plotDir, fileName)
586+
testFileName := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path))
587+
plotDir := datapathutils.TestDataPath(t, "generated", testFileName)
588+
589+
// To ensure the plots are up to date, the datapoints are hashed and
590+
// printed in the output.
591+
hasher := fnv.New64a()
592+
_, err := hasher.Write([]byte(fmt.Sprint(ts[stat])))
593+
require.NoError(t, err)
594+
595+
plotFileName := fmt.Sprintf("%s_%d_%s.png", testFileName, sample, stat)
606596

607597
var rewrite bool
608598
require.NoError(t, sniffarg.DoEnv("rewrite", &rewrite))
609599
if rewrite {
610-
p := plot.New()
611-
p.Title.Text = stat
612-
p.X.Label.Text = "Ticks"
613-
p.Y.Label.Text = stat
614-
615-
var plotArgs []interface{}
616-
for storeIdx, storeTS := range ts[stat] {
617-
storeID := storeIdx + 1
618-
pts := make(plotter.XYs, len(storeTS))
619-
for i, val := range storeTS {
620-
pts[i].X = float64(i) // tick
621-
pts[i].Y = val
622-
}
623-
plotArgs = append(plotArgs, fmt.Sprintf("store-%d", storeID), pts)
624-
}
625-
626-
err := plotutil.AddLinePoints(p, plotArgs...)
627-
require.NoError(t, err)
628-
require.NoError(t, p.Save(8*vg.Inch, 6*vg.Inch, filePath))
600+
_ = os.MkdirAll(plotDir, 0755)
601+
plotPath := filepath.Join(plotDir, plotFileName)
602+
b := generatePlot(t, stat, ts[stat])
603+
require.NoError(t, os.WriteFile(plotPath, b, 0644))
629604
}
630605

631-
f, err := os.Open(filePath)
632-
require.NoError(t, err, "cannot open plot %s; use -rewrite", fileName)
633-
defer f.Close()
634-
635-
// To ensure the plots are up to date, the file contents are hashed and
636-
// printed in the output.
637-
hasher := fnv.New64a()
638-
_, err = io.Copy(hasher, f)
639-
require.NoError(t, err)
640-
buf.WriteString(fmt.Sprintf("%s (%x)", fileName, hasher.Sum(nil)))
606+
buf.WriteString(fmt.Sprintf("%s (%x)", plotFileName, hasher.Sum(nil)))
641607
return buf.String()
642608
default:
643609
return fmt.Sprintf("unknown command: %s", d.Cmd)
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package tests
7+
8+
import (
9+
"bytes"
10+
"fmt"
11+
"testing"
12+
13+
"github.com/stretchr/testify/require"
14+
"gonum.org/v1/plot"
15+
"gonum.org/v1/plot/plotter"
16+
"gonum.org/v1/plot/plotutil"
17+
"gonum.org/v1/plot/vg"
18+
)
19+
20+
// generatePlot creates a single plot for the given stat from simulation history.
21+
// Hashes the plot file content directly to the provided hasher.
22+
// If rewrite is false, the plot is generated but not saved to disk.
23+
// Returns the filename or an error if plot generation fails.
24+
func generatePlot(t *testing.T, stat string, sl [][]float64) []byte {
25+
p := plot.New()
26+
p.Title.Text = stat
27+
// TODO(tbg): convert to time.
28+
p.X.Label.Text = "Ticks"
29+
p.Y.Label.Text = stat
30+
31+
var plotArgs []interface{}
32+
for storeIdx, storeTS := range sl {
33+
storeID := storeIdx + 1
34+
pts := make(plotter.XYs, len(storeTS))
35+
for i, val := range storeTS {
36+
pts[i].X = float64(i) // tick
37+
pts[i].Y = val
38+
}
39+
plotArgs = append(plotArgs, fmt.Sprintf("s%d", storeID), pts)
40+
}
41+
42+
err := plotutil.AddLinePoints(p, plotArgs...)
43+
require.NoError(t, err)
44+
wt, err := p.WriterTo(8*vg.Inch, 6*vg.Inch, "png")
45+
require.NoError(t, err)
46+
var buf bytes.Buffer
47+
_, err = wt.WriteTo(&buf)
48+
require.NoError(t, err)
49+
return buf.Bytes()
50+
}
32.6 KB
46.2 KB
62.4 KB
47.5 KB
38.7 KB
54 KB
55.3 KB

0 commit comments

Comments
 (0)