chore: Improve testJSONMarshal

[alexandear]

This PR rewrites testJSONMarshal, so that it properly verifies field marshaling and unmarshaling. Unmarshaling is more complex, as it requires adding multiple options to work around limitations in some of our structs.

I removed tests for structs with only use url field tags, since they are unnecessary.

As #3519 has had no activity since April, I decided to complete this work.

Original author: @exageraldo.
Applied review comments from: @stevehipwell

Fixes #2699
Closes #3519

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.08%. Comparing base (5f68d54) to head (1ebe1d7).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4042   +/-   ##
=======================================
  Coverage   94.08%   94.08%           
=======================================
  Files         207      207           
  Lines       19217    19217           
=======================================
  Hits        18081    18081           
  Misses        938      938           
  Partials      198      198           

☔ 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]

Thank you, @alexandear, this looks great!
I have just a few nits/questions for you, otherwise LGTM.
After that, we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @zyfy29 - @Not-Dhananjay-Mishra

[gmlewis]

So moving forward, do developers need to add testJSONMarshal or testJSONMarshalData or testJSONUnmarshalData or a combination of the latter, or how will they decide?

[alexandear]

So moving forward, do developers need to add testJSONMarshal or testJSONMarshalData or testJSONUnmarshalData or a combination of the latter, or how will they decide?

That's a good question, and it highlights a somewhat misleading API. I've slightly improved the test functions to make their intent clearer.

In general, developers should use testJSONMarshal, and only in rare cases use testJSONMarshalOnly or testJSONUnmarshalOnly.

[gmlewis]

So moving forward, do developers need to add testJSONMarshal or testJSONMarshalData or testJSONUnmarshalData or a combination of the latter, or how will they decide?

That's a good question, and it highlights a somewhat misleading API. I've slightly improved the test functions to make their intent clearer.

In general, developers should use testJSONMarshal, and only in rare cases use testJSONMarshalOnly or testJSONUnmarshalOnly.

Do you want to give some guidance or recommendations in either CONTRIBUTING.md or in the comments of these functions?

[gmlewis]

Excellent! Thank you, @alexandear!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.

cc: @stevehipwell - @zyfy29 - @Not-Dhananjay-Mishra