Improve testJSONMarshal

[exageraldo]

Fixes: #2699

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.24%. Comparing base (6a7684f) to head (0ded469).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3519   +/-   ##
=======================================
  Coverage   91.23%   91.24%           
=======================================
  Files         183      183           
  Lines       16053    16063   +10     
=======================================
+ Hits        14646    14656   +10     
  Misses       1231     1231           
  Partials      176      176           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[gmlewis]

@exageraldo - is this still a draft PR? If not, can you please remove the draft status?

[gmlewis]

Thank you, @exageraldo!
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@stevehipwell - might you have time for a code review? Thank you!

[stevehipwell]

Thanks for the PR @exageraldo, I'm familiar with this issue as I had to implement my own solutions (marshal & unmarshal) for this when I refactored the rules and a global fix would be great!

Based on my experience think your implementation of testJSONMarshal is an improvement but still doesn't quite keep the marshal and unmarshal concerns separate. I think a function to test both marshal and unmarshal independently would look something like this.

func testJSONMarshal(t *testing.T, v any, want string) {
	t.Helper()

	gotBytes, err := json.Marshal(v)
	if err != nil {
		t.Errorf("Unable to marshal JSON for %#v", v)
	}

	if diff := cmp.Diff(want, string(gotBytes)); diff != "" {
		t.Errorf(
			"json.Marshal returned:\n%s\nwant:\n%s\ndiff:\n%v",
			gotBytes,
			want,
			diff,
		)
	}

	var gotAny any
	err = json.Unmarshal([]byte(want), gotAny)
	if err != nil {
		t.Errorf("Unable to unmarshal JSON %v: %v", want, err)
	}

	if diff := cmp.Diff(v, gotAny); diff != "" {
		t.Errorf(
			"json.Unmarshal returned:\n%#v\nwant:\n%#v\ndiff:\n%v",
			gotAny,
			v,
			diff,
		)
	}
}

If this was greenfield I'd suggest splitting the test methods up and have a seperate test method for marshal and unmarshal, but the above signature matches the current state of play.