Skip to content

Commit

Permalink
cmd: test command file output improvements (#3384)
Browse files Browse the repository at this point in the history 
Couple of small improvements over the test command:

- Swap TOML with JSON file on output
- Change the file format to include all 5 categories, so commands like `test all` can successfully write all results to the file
- Safe write to the file (using temp file)
- Put the MEV create block tests under a `--load-test` flag, so we don't put external infra under excess in the default case

An some small fixes that have been missed:

- Rename the testperformance file to testinfra
- Add the beacon node simulations file directory for results as a flag

category: refactor
ticket: none
  • Loading branch information
@KaloyanTanev
KaloyanTanev authored Nov 15, 2024
1 parent 8c95d9d commit eba4d33
Show file tree
Hide file tree
Showing 13 changed files with 193 additions and 101 deletions.
98 changes: 78 additions & 20 deletions cmd/test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,21 @@ package cmd

import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptrace"
"os"
"os/signal"
"path/filepath"
"slices"
"sort"
"strings"
"syscall"
"time"
"unicode/utf8"

"github.com/pelletier/go-toml/v2"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"golang.org/x/exp/maps"
Expand Down Expand Up @@ -48,7 +49,7 @@ const (
)

type testConfig struct {
OutputToml string
OutputJSON string
Quiet bool
TestCases []string
Timeout time.Duration
Expand All @@ -67,7 +68,7 @@ func newTestCmd(cmds ...*cobra.Command) *cobra.Command {
}

func bindTestFlags(cmd *cobra.Command, config *testConfig) {
cmd.Flags().StringVar(&config.OutputToml, "output-toml", "", "File path to which output can be written in TOML format.")
cmd.Flags().StringVar(&config.OutputJSON, "output-json", "", "File path to which output can be written in JSON format.")
cmd.Flags().StringSliceVar(&config.TestCases, "test-cases", nil, fmt.Sprintf("List of comma separated names of tests to be exeucted. Available tests are: %v", listTestCases(cmd)))
cmd.Flags().DurationVar(&config.Timeout, "timeout", time.Hour, "Execution timeout for all tests.")
cmd.Flags().BoolVar(&config.Quiet, "quiet", false, "Do not print test results to stdout.")
Expand Down Expand Up @@ -117,8 +118,8 @@ func listTestCases(cmd *cobra.Command) []string {
}

func mustOutputToFileOnQuiet(cmd *cobra.Command) error {
if cmd.Flag("quiet").Changed && !cmd.Flag("output-toml").Changed {
return errors.New("on --quiet, an --output-toml is required")
if cmd.Flag("quiet").Changed && !cmd.Flag("output-json").Changed {
return errors.New("on --quiet, an --output-json is required")
}

return nil
Expand Down Expand Up @@ -150,16 +151,15 @@ const (
categoryScoreC categoryScore = "C"
)

// toml fails on marshaling errors to string, so we wrap the errors and add custom marshal
type testResultError struct{ error }

type testResult struct {
Name string
Verdict testVerdict
Measurement string
Suggestion string
Error testResultError
IsAcceptable bool
Name string `json:"name"`
Verdict testVerdict `json:"verdict"`
Measurement string `json:"measurement,omitempty"`
Suggestion string `json:"suggestion,omitempty"`
Error testResultError `json:"error,omitempty"`
IsAcceptable bool `json:"-"`
}

func failedTestResult(testRes testResult, err error) testResult {
Expand Down Expand Up @@ -198,10 +198,10 @@ type testCaseName struct {
}

type testCategoryResult struct {
CategoryName string
Targets map[string][]testResult
ExecutionTime Duration
Score categoryScore
CategoryName string `json:"category_name,omitempty"`
Targets map[string][]testResult `json:"targets,omitempty"`
ExecutionTime Duration `json:"execution_time,omitempty"`
Score categoryScore `json:"score,omitempty"`
}

func appendScore(cat []string, score []string) []string {
Expand All @@ -213,15 +213,73 @@ func appendScore(cat []string, score []string) []string {
return res
}

type fileResult struct {
Peers testCategoryResult `json:"charon_peers,omitempty"`
Beacon testCategoryResult `json:"beacon_node,omitempty"`
Validator testCategoryResult `json:"validator_client,omitempty"`
MEV testCategoryResult `json:"mev,omitempty"`
Infra testCategoryResult `json:"infra,omitempty"`
}

func writeResultToFile(res testCategoryResult, path string) error {
f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o444)
// open or create a file
existingFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o644)
if err != nil {
return errors.Wrap(err, "create/open file")
}
defer f.Close()
err = toml.NewEncoder(f).Encode(res)
defer existingFile.Close()
stat, err := existingFile.Stat()
if err != nil {
return errors.Wrap(err, "get file stat")
}
// read file contents or default to empty structure
var file fileResult
if stat.Size() == 0 {
file = fileResult{}
} else {
err = json.NewDecoder(existingFile).Decode(&file)
if err != nil {
return errors.Wrap(err, "decode fileResult from JSON")
}
}

switch res.CategoryName {
case peersTestCategory:
file.Peers = res
case beaconTestCategory:
file.Beacon = res
case validatorTestCategory:
file.Validator = res
case mevTestCategory:
file.MEV = res
case infraTestCategory:
file.Infra = res
}

// write data to temp file
tmpFile, err := os.CreateTemp(filepath.Dir(path), fmt.Sprintf("%v-tmp-*.json", filepath.Base(path)))
if err != nil {
return errors.Wrap(err, "create temp file")
}
defer tmpFile.Close()
err = tmpFile.Chmod(0o644)
if err != nil {
return errors.Wrap(err, "chmod temp file")
}

fileContentJSON, err := json.Marshal(file)
if err != nil {
return errors.Wrap(err, "marshal fileResult to JSON")
}

_, err = tmpFile.Write(fileContentJSON)
if err != nil {
return errors.Wrap(err, "write json to file")
}

err = os.Rename(tmpFile.Name(), path)
if err != nil {
return errors.Wrap(err, "encode testCategoryResult to TOML")
return errors.Wrap(err, "rename temp file")
}

return nil
Expand Down
8 changes: 4 additions & 4 deletions cmd/testbeacon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ func bindTestBeaconFlags(cmd *cobra.Command, config *testBeaconConfig, flagsPref
cmd.Flags().BoolVar(&config.LoadTest, flagsPrefix+"load-test", false, "Enable load test, not advisable when testing towards external beacon nodes.")
cmd.Flags().DurationVar(&config.LoadTestDuration, flagsPrefix+"load-test-duration", 5*time.Second, "Time to keep running the load tests in seconds. For each second a new continuous ping instance is spawned.")
cmd.Flags().IntVar(&config.SimulationDuration, flagsPrefix+"simulation-duration-in-slots", slotsInEpoch, "Time to keep running the simulation in slots.")
cmd.Flags().StringVar(&config.SimulationFileDir, flagsPrefix+"simulation-file-dir", "./", "Time to keep running the simulation in slots.")
cmd.Flags().BoolVar(&config.SimulationVerbose, flagsPrefix+"simulation-verbose", false, "Show results for each request and each validator.")
mustMarkFlagRequired(cmd, flagsPrefix+"endpoints")
}
Expand Down Expand Up @@ -250,8 +251,8 @@ func runTestBeacon(ctx context.Context, w io.Writer, cfg testBeaconConfig) (err
}
}

if cfg.OutputToml != "" {
err = writeResultToFile(res, cfg.OutputToml)
if cfg.OutputJSON != "" {
err = writeResultToFile(res, cfg.OutputJSON)
if err != nil {
return err
}
Expand All @@ -270,9 +271,8 @@ func testAllBeacons(ctx context.Context, queuedTestCases []testCaseName, allTest
group, _ := errgroup.WithContext(ctx)

for _, endpoint := range conf.Endpoints {
currEndpoint := endpoint // TODO: can be removed after go1.22 version bump
group.Go(func() error {
return testSingleBeacon(ctx, queuedTestCases, allTestCases, conf, currEndpoint, singleBeaconResCh)
return testSingleBeacon(ctx, queuedTestCases, allTestCases, conf, endpoint, singleBeaconResCh)
})
}

Expand Down
24 changes: 12 additions & 12 deletions cmd/testbeacon_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestBeaconTest(t *testing.T) {
name: "default scenario",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: nil,
Timeout: time.Minute,
Expand Down Expand Up @@ -71,7 +71,7 @@ func TestBeaconTest(t *testing.T) {
name: "connection refused",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: nil,
Timeout: time.Minute,
Expand All @@ -87,7 +87,7 @@ func TestBeaconTest(t *testing.T) {
name: "timeout",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: nil,
Timeout: 100 * time.Nanosecond,
Expand All @@ -110,7 +110,7 @@ func TestBeaconTest(t *testing.T) {
name: "quiet",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: true,
TestCases: nil,
Timeout: time.Minute,
Expand All @@ -126,7 +126,7 @@ func TestBeaconTest(t *testing.T) {
name: "unsupported test",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: []string{"notSupportedTest"},
Timeout: time.Minute,
Expand All @@ -140,7 +140,7 @@ func TestBeaconTest(t *testing.T) {
name: "custom test cases",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: []string{"ping"},
Timeout: time.Minute,
Expand All @@ -163,7 +163,7 @@ func TestBeaconTest(t *testing.T) {
name: "write to file",
config: testBeaconConfig{
testConfig: testConfig{
OutputToml: "./write-to-file-test.toml.tmp",
OutputJSON: "./write-to-file-test.json.tmp",
Quiet: false,
TestCases: nil,
Timeout: time.Minute,
Expand Down Expand Up @@ -196,7 +196,7 @@ func TestBeaconTest(t *testing.T) {
}
defer func() {
if test.cleanup != nil {
test.cleanup(t, test.config.OutputToml)
test.cleanup(t, test.config.OutputJSON)
}
}()

Expand All @@ -206,8 +206,8 @@ func TestBeaconTest(t *testing.T) {
testWriteOut(t, test.expected, buf)
}

if test.config.OutputToml != "" {
testWriteFile(t, test.expected, test.config.OutputToml)
if test.config.OutputJSON != "" {
testWriteFile(t, test.expected, test.config.OutputJSON)
}
})
}
Expand Down Expand Up @@ -281,9 +281,9 @@ func TestBeaconTestFlags(t *testing.T) {
expectedErr: "required flag(s) \"endpoints\" not set",
},
{
name: "no output toml on quiet",
name: "no output json on quiet",
args: []string{"beacon", "--endpoints=\"test.endpoint\"", "--quiet"},
expectedErr: "on --quiet, an --output-toml is required",
expectedErr: "on --quiet, an --output-json is required",
},
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/testperformance.go → cmd/testinfra.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ func runTestInfra(ctx context.Context, w io.Writer, cfg testInfraConfig) (err er
}
}

if cfg.OutputToml != "" {
err = writeResultToFile(res, cfg.OutputToml)
if cfg.OutputJSON != "" {
err = writeResultToFile(res, cfg.OutputJSON)
if err != nil {
return err
}
Expand Down
22 changes: 11 additions & 11 deletions cmd/testperformance_internal_test.go → cmd/testinfra_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestInfraTest(t *testing.T) {
name: "default scenario",
config: testInfraConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: []string{"availableMemory", "totalMemory", "internetLatency"},
Timeout: time.Minute,
Expand All @@ -55,7 +55,7 @@ func TestInfraTest(t *testing.T) {
name: "timeout",
config: testInfraConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: nil,
Timeout: 100 * time.Nanosecond,
Expand All @@ -77,7 +77,7 @@ func TestInfraTest(t *testing.T) {
name: "quiet",
config: testInfraConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: true,
TestCases: []string{"availableMemory", "totalMemory", "internetLatency"},
Timeout: time.Minute,
Expand All @@ -101,7 +101,7 @@ func TestInfraTest(t *testing.T) {
name: "unsupported test",
config: testInfraConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: []string{"notSupportedTest"},
Timeout: time.Minute,
Expand All @@ -118,7 +118,7 @@ func TestInfraTest(t *testing.T) {
name: "custom test cases",
config: testInfraConfig{
testConfig: testConfig{
OutputToml: "",
OutputJSON: "",
Quiet: false,
TestCases: []string{"totalMemory"},
Timeout: time.Minute,
Expand All @@ -140,7 +140,7 @@ func TestInfraTest(t *testing.T) {
name: "write to file",
config: testInfraConfig{
testConfig: testConfig{
OutputToml: "./write-to-file-test.toml.tmp",
OutputJSON: "./write-to-file-test.json.tmp",
Quiet: false,
TestCases: []string{"availableMemory", "totalMemory", "internetLatency"},
Timeout: time.Minute,
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestInfraTest(t *testing.T) {
}
defer func() {
if test.cleanup != nil {
test.cleanup(t, test.config.OutputToml)
test.cleanup(t, test.config.OutputJSON)
}
}()

Expand All @@ -189,8 +189,8 @@ func TestInfraTest(t *testing.T) {
testWriteOut(t, test.expected, buf)
}

if test.config.OutputToml != "" {
testWriteFile(t, test.expected, test.config.OutputToml)
if test.config.OutputJSON != "" {
testWriteFile(t, test.expected, test.config.OutputJSON)
}
})
}
Expand Down Expand Up @@ -227,9 +227,9 @@ func TestInfraTestFlags(t *testing.T) {
expectedErr: "",
},
{
name: "no output toml on quiet",
name: "no output json on quiet",
args: []string{"infra", "--disk-io-block-size-kb=1", "--quiet"},
expectedErr: "on --quiet, an --output-toml is required",
expectedErr: "on --quiet, an --output-json is required",
},
}

Expand Down
Loading

0 comments on commit eba4d33

Please sign in to comment.