{Bool,Binary,UnixMilli}#MarshalJSON(): return valid JSON, not empty string

[Al2Klimov]

in case an instance is null.

[yhabteab]

The changes are fine, however if there were units for them from the beginning, such bugs would have been easy to spot. Thus, it would be nice if you could add the missing unit tests here as well.

[yhabteab]

I don't quite get why you're asserting the tests manually and with some type reflecting to top it all off. Why can't you just use require.* and assert.*?

[Al2Klimov]

I copy&paste-d existing pkg/icingadb/objectpacker/objectpacker_test.go

[yhabteab]

I copy&paste-d existing pkg/icingadb/objectpacker/objectpacker_test.go

And why does it have to be exactly the same here? If you just want to extend the error messages and add some contexts in case of errors, you can do that with require.NoError(t, err, "test failed") as well. I don't see any advantage in checking the results manually with reflect.* and t.Errorf().

[yhabteab]

LGTM!

[julianbrost]

Please give the sub-tests proper names. The spaces you use are not valid in test names so they are already replaces with _ and the special characters make it annoying to use with something like go test -run, which takes regexes, so escaping is needed:

go test -v -run 'TestUnixMilli_MarshalJSON/0001-01-01_00:00:00_\+0000_UTC_\(Unix:_-62135596800\)' ./pkg/types