simulators/ethereum/rpc-compat: handle test comments and avoid error message comparison (#984)

Author: fjl

simulators/ethereum/rpc-compat/go.mod

@@ -25,6 +25,10 @@ require (
 	github.com/sergi/go-diff v1.2.0 // indirect
 	github.com/shirou/gopsutil v3.21.11+incompatible // indirect
 	github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a // indirect
+	github.com/tidwall/gjson v1.17.0 // indirect
+	github.com/tidwall/match v1.1.1 // indirect
+	github.com/tidwall/pretty v1.2.0 // indirect
+	github.com/tidwall/sjson v1.2.5 // indirect
 	github.com/tklauser/go-sysconf v0.3.12 // indirect
 	github.com/tklauser/numcpus v0.6.1 // indirect
 	github.com/yudai/golcs v0.0.0-20170316035057-ecda9a501e82 // indirect

simulators/ethereum/rpc-compat/go.sum

@@ -90,6 +90,16 @@ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1F
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
 github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a h1:1ur3QoCqvE5fl+nylMaIr9PVV1w343YRDtsy+Rwu7XI=
 github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a/go.mod h1:RRCYJbIwD5jmqPI9XoAFR0OcDxqUctll6zUj/+B4S48=
+github.com/tidwall/gjson v1.14.2 h1:6BBkirS0rAHjumnjHF6qgy5d2YAJ1TLIaFE2lzfOLqo=
+github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
+github.com/tidwall/gjson v1.17.0 h1:/Jocvlh98kcTfpN2+JzGQWQcqrPQwDrVEMApx/M5ZwM=
+github.com/tidwall/gjson v1.17.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
+github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
+github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
+github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs=
+github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
+github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
+github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
 github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFAEVmqU=
 github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI=
 github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk=

simulators/ethereum/rpc-compat/main.go

@@ -7,14 +7,14 @@ import (
 	"io"
 	"net"
 	"net/http"
-	"os"
-	"path/filepath"
 	"regexp"
 	"strings"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/hive/hivesim"
+	"github.com/tidwall/gjson"
+	"github.com/tidwall/sjson"
 	diff "github.com/yudai/gojsondiff"
 	"github.com/yudai/gojsondiff/formatter"
 )
@@ -26,11 +26,6 @@ var (
 	}
 )
 
-type test struct {
-	Name string
-	Data []byte
-}
-
 func main() {
 	// Load fork environment.
 	var clientEnv hivesim.Params
@@ -66,63 +61,71 @@ conformance with the execution API specification.`[1:],
 func runAllTests(t *hivesim.T, c *hivesim.Client, clientName string) {
 	_, testPattern := t.Sim.TestPattern()
 	re := regexp.MustCompile(testPattern)
-
 	tests := loadTests(t, "tests", re)
 	for _, test := range tests {
+		test := test
 		t.Run(hivesim.TestSpec{
-			Name: fmt.Sprintf("%s (%s)", test.Name, clientName),
+			Name:        fmt.Sprintf("%s (%s)", test.name, clientName),
+			Description: test.comment,
 			Run: func(t *hivesim.T) {
-				if err := runTest(t, c, test.Data); err != nil {
+				if err := runTest(t, c, &test); err != nil {
 					t.Fatal(err)
 				}
 			},
 		})
 	}
 }
 
-func runTest(t *hivesim.T, c *hivesim.Client, data []byte) error {
+func runTest(t *hivesim.T, c *hivesim.Client, test *rpcTest) error {
 	var (
-		client = &http.Client{
-			Timeout: 5 * time.Second,
-			Transport: &loggingRoundTrip{
-				t:     t,
-				inner: http.DefaultTransport,
-			},
-		}
-		url  = fmt.Sprintf("http://%s", net.JoinHostPort(c.IP.String(), "8545"))
-		err  error
-		resp []byte
+		client    = &http.Client{Timeout: 5 * time.Second}
+		url       = fmt.Sprintf("http://%s", net.JoinHostPort(c.IP.String(), "8545"))
+		err       error
+		respBytes []byte
 	)
 
-	for _, line := range strings.Split(string(data), "\n") {
-		line = strings.TrimSpace(line)
-		switch {
-		case len(line) == 0 || strings.HasPrefix(line, "//"):
-			// Skip comments, blank lines.
-			continue
-		case strings.HasPrefix(line, ">> "):
+	for _, msg := range test.messages {
+		if msg.send {
 			// Send request.
-			resp, err = postHttp(client, url, []byte(line[3:]))
+			t.Log(">> ", msg.data)
+			respBytes, err = postHttp(client, url, strings.NewReader(msg.data))
 			if err != nil {
 				return err
 			}
-		case strings.HasPrefix(line, "<< "):
-			// Read response. Unmarshal to interface{} to verify deep equality. Marshal
-			// again to remove padding differences and to print each field in the same
-			// order. This makes it easy to spot any discrepancies.
-			if resp == nil {
+		} else {
+			// Receive a response.
+			if respBytes == nil {
 				return fmt.Errorf("invalid test, response before request")
 			}
-			want := []byte(strings.TrimSpace(line)[3:]) // trim leading "<< "
-			// Now compare.
-			d, err := diff.New().Compare(resp, want)
+			expectedData := msg.data
+			resp := string(bytes.TrimSpace(respBytes))
+			t.Log("<< ", resp)
+			if !gjson.Valid(resp) {
+				return fmt.Errorf("invalid JSON response")
+			}
+
+			// Patch JSON to remove error messages. We only do this in the specific case
+			// where an error is expected AND returned by the client.
+			var errorRedacted bool
+			if gjson.Get(resp, "error").Exists() && gjson.Get(expectedData, "error").Exists() {
+				resp, _ = sjson.Delete(resp, "error.message")
+				expectedData, _ = sjson.Delete(expectedData, "error.message")
+				errorRedacted = true
+			}
+
+			// Compare responses.
+			d, err := diff.New().Compare([]byte(resp), []byte(expectedData))
 			if err != nil {
 				return fmt.Errorf("failed to unmarshal value: %s\n", err)
 			}
+
 			// If there is a discrepancy, return error.
 			if d.Modified() {
-				var got map[string]interface{}
-				json.Unmarshal(resp, &got)
+				if errorRedacted {
+					t.Log("note: error messages removed from comparison")
+				}
+				var got map[string]any
+				json.Unmarshal([]byte(resp), &got)
 				config := formatter.AsciiFormatterConfig{
 					ShowArrayIndex: true,
 					Coloring:       false,
@@ -131,22 +134,20 @@ func runTest(t *hivesim.T, c *hivesim.Client, data []byte) error {
 				diffString, _ := formatter.Format(d)
 				return fmt.Errorf("response differs from expected (-- client, ++ test):\n%s", diffString)
 			}
-			resp = nil
-		default:
-			t.Fatalf("invalid line in test script: %s", line)
+			respBytes = nil
 		}
 	}
-	if resp != nil {
+
+	if respBytes != nil {
 		t.Fatalf("unhandled response in test case")
 	}
 	return nil
 }
 
 // sendHttp sends an HTTP POST with the provided json data and reads the
 // response into a byte slice and returns it.
-func postHttp(c *http.Client, url string, d []byte) ([]byte, error) {
-	data := bytes.NewBuffer(d)
-	req, err := http.NewRequest("POST", url, data)
+func postHttp(c *http.Client, url string, d io.Reader) ([]byte, error) {
+	req, err := http.NewRequest("POST", url, d)
 	if err != nil {
 		return nil, fmt.Errorf("error building request: %v", err)
 	}
@@ -158,35 +159,7 @@ func postHttp(c *http.Client, url string, d []byte) ([]byte, error) {
 	return io.ReadAll(resp.Body)
 }
 
-// loadTests walks the given directory looking for *.io files to load.
-func loadTests(t *hivesim.T, root string, re *regexp.Regexp) []test {
-	tests := make([]test, 0)
-	filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
-		if err != nil {
-			t.Logf("unable to walk path: %s", err)
-			return err
-		}
-		if info.IsDir() {
-			return nil
-		}
-		if fname := info.Name(); !strings.HasSuffix(fname, ".io") {
-			return nil
-		}
-		pathname := strings.TrimSuffix(strings.TrimPrefix(path, root), ".io")
-		if !re.MatchString(pathname) {
-			fmt.Println("skip", pathname)
-			return nil // skip
-		}
-		data, err := os.ReadFile(path)
-		if err != nil {
-			return err
-		}
-		tests = append(tests, test{strings.TrimLeft(pathname, "/"), data})
-		return nil
-	})
-	return tests
-}
-
+// sendForkchoiceUpdated delivers the initial FcU request to the client.
 func sendForkchoiceUpdated(t *hivesim.T, client *hivesim.Client) {
 	var request struct {
 		Method string
@@ -195,43 +168,11 @@ func sendForkchoiceUpdated(t *hivesim.T, client *hivesim.Client) {
 	if err := common.LoadJSON("tests/headfcu.json", &request); err != nil {
 		t.Fatal("error loading forkchoiceUpdated:", err)
 	}
-	err := client.EngineAPI().Call(nil, request.Method, request.Params...)
+	t.Logf("sending %s: %v", request.Method, request.Params)
+	var resp any
+	err := client.EngineAPI().Call(&resp, request.Method, request.Params...)
 	if err != nil {
 		t.Fatal("client rejected forkchoiceUpdated:", err)
 	}
-}
-
-// loggingRoundTrip writes requests and responses to the test log.
-type loggingRoundTrip struct {
-	t     *hivesim.T
-	inner http.RoundTripper
-}
-
-func (rt *loggingRoundTrip) RoundTrip(req *http.Request) (*http.Response, error) {
-	// Read and log the request body.
-	reqBytes, err := io.ReadAll(req.Body)
-	req.Body.Close()
-	if err != nil {
-		return nil, err
-	}
-	rt.t.Logf(">>  %s", bytes.TrimSpace(reqBytes))
-	reqCopy := *req
-	reqCopy.Body = io.NopCloser(bytes.NewReader(reqBytes))
-
-	// Do the round trip.
-	resp, err := rt.inner.RoundTrip(&reqCopy)
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	// Read and log the response bytes.
-	respBytes, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
-	}
-	respCopy := *resp
-	respCopy.Body = io.NopCloser(bytes.NewReader(respBytes))
-	rt.t.Logf("<<  %s", bytes.TrimSpace(respBytes))
-	return &respCopy, nil
+	t.Logf("response: %v", resp)
 }

simulators/ethereum/rpc-compat/testload.go

@@ -0,0 +1,102 @@
+package main
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"regexp"
+	"strings"
+
+	"github.com/ethereum/hive/hivesim"
+	"github.com/tidwall/gjson"
+)
+
+type rpcTest struct {
+	name     string
+	comment  string
+	speconly bool
+	messages []rpcTestMessage
+}
+
+type rpcTestMessage struct {
+	data string
+	// if true, the message is a send (>>), otherwise it's a receive (<<)
+	send bool
+}
+
+func loadTestFile(name string, r io.Reader) (rpcTest, error) {
+	var (
+		rdr      = bufio.NewReader(r)
+		scan     = bufio.NewScanner(rdr)
+		inHeader = true
+		test     = rpcTest{name: name}
+	)
+	for scan.Scan() {
+		line := strings.TrimSpace(scan.Text())
+		switch {
+		case len(line) == 0:
+			continue
+
+		case strings.HasPrefix(line, "//"):
+			if !inHeader {
+				continue // ignore comments after requests
+			}
+			text := strings.TrimPrefix(strings.TrimPrefix(line, "//"), " ")
+			test.comment += text + "\n"
+			if strings.HasPrefix(text, "speconly:") {
+				test.speconly = true
+			}
+
+		case strings.HasPrefix(line, ">>") || strings.HasPrefix(line, "<<"):
+			inHeader = false
+			data := strings.TrimSpace(line[2:])
+			if !gjson.Valid(data) {
+				return test, fmt.Errorf("invalid JSON in line %q", line)
+			}
+			test.messages = append(test.messages, rpcTestMessage{
+				data: data,
+				send: strings.HasPrefix(line, ">>"),
+			})
+
+		default:
+			return test, fmt.Errorf("invalid test line: %q", line)
+		}
+	}
+	return test, scan.Err()
+}
+
+// loadTests walks the given directory looking for *.io files to load.
+func loadTests(t *hivesim.T, root string, re *regexp.Regexp) []rpcTest {
+	var tests []rpcTest
+	filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			t.Logf("unable to walk path: %s", err)
+			return err
+		}
+		if info.IsDir() {
+			return nil
+		}
+		if fname := info.Name(); !strings.HasSuffix(fname, ".io") {
+			return nil
+		}
+		pathname := strings.TrimSuffix(strings.TrimPrefix(path, root+"/"), ".io")
+		if !re.MatchString(pathname) {
+			fmt.Println("skip", pathname)
+			return nil // skip
+		}
+		fd, err := os.Open(path)
+		if err != nil {
+			return err
+		}
+		defer fd.Close()
+		test, err := loadTestFile(pathname, fd)
+		if err != nil {
+			return fmt.Errorf("invalid test %s: %v", info.Name(), err)
+		}
+		tests = append(tests, test)
+		return nil
+	})
+	return tests
+}

simulators/ethereum/rpc-compat/testload_test.go

@@ -0,0 +1,48 @@
+package main
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+func TestLoad(t *testing.T) {
+	data := `// this is a test comment
+// this is the second line
+// speconly: lalalala
+>> {"type":"send"}
+<< {"type":"recv"}
+`
+
+	expectedComment := `this is a test comment
+this is the second line
+speconly: lalalala
+`
+	expectedMessages := []rpcTestMessage{
+		{
+			data: `{"type":"send"}`,
+			send: true,
+		},
+		{
+			data: `{"type":"recv"}`,
+			send: false,
+		},
+	}
+
+	result, err := loadTestFile("the-test", strings.NewReader(data))
+	if err != nil {
+		t.Fatal("error:", err)
+	}
+	if result.name != "the-test" {
+		t.Error("wrong test name:", result.comment)
+	}
+	if result.comment != expectedComment {
+		t.Errorf("wrong test comment %q", result.comment)
+	}
+	if !result.speconly {
+		t.Error("test is not marked speconly")
+	}
+	if !reflect.DeepEqual(result.messages, expectedMessages) {
+		t.Errorf("wrong test messages %+v", result.messages)
+	}
+}