Copilot commented on code in PR #1114:
URL:
https://github.com/apache/incubator-seata-go/pull/1114#discussion_r3141628326
##########
pkg/util/flagext/day_test.go:
##########
@@ -63,6 +64,36 @@ func TestDayValueYAML(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, expected, actual)
+ var actualStruct TestStruct
+ err = yaml.Unmarshal(expected, &actualStruct)
+ require.NoError(t, err)
+ assert.Equal(t, testStruct, actualStruct)
+ }
+ // Test UTC-stable string and YAML serialization in western timezones.
+ {
+ loc, err := time.LoadLocation("America/Los_Angeles")
+ if err != nil {
+ loc = time.FixedZone("UTC-8", -8*60*60)
+ }
+
+ originalLocal := time.Local
+ time.Local = loc
+ defer func() {
+ time.Local = originalLocal
+ }()
+ type TestStruct struct {
+ Day *DayValue `yaml:"day"`
+ }
+ var testStruct TestStruct
+ testStruct.Day = &DayValue{}
+ require.NoError(t, testStruct.Day.Set("1985-06-02"))
+ expected := []byte(`day: "1985-06-02"
+`)
+
+ actual, err := yaml.Marshal(testStruct)
+ require.NoError(t, err)
+ assert.Equal(t, expected, actual)
Review Comment:
This new timezone-specific YAML test duplicates the prior "pointers of
DayValue" block almost line-for-line. Consider making the pointer test
table-driven (e.g., parameterize the location) or extracting a small helper to
reduce duplication and make future additions (more timezones) easier.
##########
pkg/util/flagext/day_test.go:
##########
@@ -63,6 +64,36 @@ func TestDayValueYAML(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, expected, actual)
+ var actualStruct TestStruct
+ err = yaml.Unmarshal(expected, &actualStruct)
+ require.NoError(t, err)
+ assert.Equal(t, testStruct, actualStruct)
+ }
+ // Test UTC-stable string and YAML serialization in western timezones.
+ {
+ loc, err := time.LoadLocation("America/Los_Angeles")
+ if err != nil {
Review Comment:
The comment says this block tests "UTC-stable string and YAML
serialization", but the test only asserts YAML marshal/unmarshal output and
never checks DayValue.String(). Either add an assertion for the String() output
under the modified time.Local, or adjust the comment to only mention YAML
serialization to keep the test intent accurate.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]