From 8babc5e1722ba2f2954146e6a56c14a4143d39f5 Mon Sep 17 00:00:00 2001 From: Martin Petkov Date: Mon, 3 May 2021 15:56:11 -0400 Subject: [PATCH] fix(cli): Test stringViaJSON structure, not exact string (#909) In github.com/GoogleCloudPlatform/cloud-foundation-toolkit/issues/902, I suggested eventually moving to protojson. According to https://github.com/golang/protobuf/issues/1121#issuecomment-627554847, that library does not support a stable serialization output. That means that the test in proto_test.go would become flaky. The suggestion there is "Instead of syntactically comparing the serialized form of a message, the test should semantically compare the structured form of a message", so I've done that here by loading the got and want strings as JSON and comparing the structured JSON. --- cli/go.mod | 1 + cli/scorecard/proto_test.go | 21 ++++++++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/cli/go.mod b/cli/go.mod index 7f015e5101a..8bf8886b762 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -7,6 +7,7 @@ require ( github.com/briandowns/spinner v1.6.1 github.com/forseti-security/config-validator v0.0.0-20200505040130-17dc60b21dc8 github.com/golang/protobuf v1.3.3 + github.com/google/go-cmp v0.3.1 // indirect github.com/hashicorp/terraform v0.12.2 // indirect github.com/inconshreveable/log15 v0.0.0-20180818164646-67afb5ed74ec github.com/open-policy-agent/opa v0.17.2 diff --git a/cli/scorecard/proto_test.go b/cli/scorecard/proto_test.go index 9418df469c8..acce9734ea8 100644 --- a/cli/scorecard/proto_test.go +++ b/cli/scorecard/proto_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/forseti-security/config-validator/pkg/api/validator" + "github.com/google/go-cmp/cmp" ) func jsonToInterface(jsonStr string) map[string]interface{} { @@ -58,13 +59,27 @@ func TestDataTypeTransformation(t *testing.T) { } }) t.Run("stringViaJSON", func(t *testing.T) { + // Compare as structured JSON objects, since eventually this + // should use the protojson package, which does not support + // stable serialization. See + // https://github.com/golang/protobuf/issues/1121#issuecomment-627554847 gotStr, err := stringViaJSON(pbAsset) - wantedStr := `{"name":"//cloudresourcemanager.googleapis.com/projects/23456","assetType":"cloudresourcemanager.googleapis.com/Project","iamPolicy":{"version":1,"bindings":[{"role":"roles/owner","members":["user:user@example.com"]}],"etag":"WwAA1Aaa/BA="},"ancestors":["projects/1234","organizations/56789"]}` if err != nil { t.Fatal("unexpected error", err) } - if gotStr != wantedStr { - t.Errorf("wanted %s, got %s", wantedStr, gotStr) + var gotJSON map[string]interface{} + if err := json.Unmarshal([]byte(gotStr), &gotJSON); err != nil { + t.Fatalf("failed to parse JSON string %v: %v", gotStr, err) + } + + wantStr := `{"name":"//cloudresourcemanager.googleapis.com/projects/23456","assetType":"cloudresourcemanager.googleapis.com/Project","iamPolicy":{"version":1,"bindings":[{"role":"roles/owner","members":["user:user@example.com"]}],"etag":"WwAA1Aaa/BA="},"ancestors":["projects/1234","organizations/56789"]}` + var wantJSON map[string]interface{} + if err := json.Unmarshal([]byte(wantStr), &wantJSON); err != nil { + t.Fatalf("failed to parse JSON string %v: %v", wantStr, err) + } + + if diff := cmp.Diff(wantJSON, gotJSON); diff != "" { + t.Errorf("stringViaJSON() returned unexpected difference (-want +got):\n%s", diff) } }) }