We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CopyTo
CopyTo is provided by several complex struct types. It allows to deep-copy instance for safety
Container.CopyTo keeps dst attributes when src has none. If dst already had some attributes, resulting copy is incorrect
Container.CopyTo
dst
src
this could be missed in 2 reasons:
correct deep copy
just do it
func TestCopyReset(t *testing.T) { var src, dst Container dst.SetAttribute("key", "val") src.CopyTo(&dst) require.Zero(t, dst.Attribute("key")) }
maybe there are other cases, review all of them. To force this, i suggest to pre-add corresponding unit tests
no (at least for noticed case)
The text was updated successfully, but these errors were encountered:
session.Container/Object have similar error in Unmarshal(JSON: context fields aint reset
session.Container/Object
Unmarshal(JSON
func TestResetContext(t *testing.T) { var c session.Container c.ForVerb(1) require.True(t, c.AssertVerb(1)) require.NoError(t, c.Unmarshal(nil)) require.False(t, c.AssertVerb(1)) }
in total, i suggest to add unit tests for resetting a previously set value for the following methods:
ReadFromV2
FromProtoMessage
Unmarshal
UnmarshalJSON
in particular cases, this can be already covered. Example
once all tests are done, some cases incl. mentioned here will FAIL. And then fix
Sorry, something went wrong.
No branches or pull requests
CopyTo
is provided by several complex struct types. It allows to deep-copy instance for safetyCurrent Behavior
Container.CopyTo
keepsdst
attributes whensrc
has none. Ifdst
already had some attributes, resulting copy is incorrectthis could be missed in 2 reasons:
CopyTo
is rarely useddst
is usually zero: np in this caseExpected Behavior
correct deep copy
Possible Solution
just do it
Steps to Reproduce
Context
maybe there are other cases, review all of them. To force this, i suggest to pre-add corresponding unit tests
Regression
no (at least for noticed case)
Your Environment
The text was updated successfully, but these errors were encountered: