Copilot commented on code in PR #1195:
URL:
https://github.com/apache/skywalking-banyandb/pull/1195#discussion_r3489462212
##########
banyand/stream/block_metadata_test.go:
##########
@@ -246,6 +246,7 @@ func Test_unmarshalBlockMetadata(t *testing.T) {
original := []blockMetadata{
{
seriesID: common.SeriesID(1),
+ tagType: make(tagType),
timestamps: timestampsMetadata{
Review Comment:
These expectations were updated to account for per-entry reset during
unmarshalling, but the test still only unmarshals into a nil destination slice.
That won’t catch the regression this PR fixes (reusing a preallocated dst via
dst[:0] can retain stale tagFamilies/tagType without resetting the reused
element). Add a subtest that pre-populates a dst slice (cap > 0) with a
blockMetadata containing non-empty maps, then call
unmarshalBlockMetadata(dst[:0], marshaled) and assert the decoded entries don’t
include the stale keys.
##########
banyand/trace/block_metadata_test.go:
##########
@@ -207,6 +207,7 @@ func Test_unmarshalBlockMetadata(t *testing.T) {
wanted := []blockMetadata{
{
traceID: "trace1",
+ tags: make(map[string]*dataBlock),
tagType: make(map[string]pbv1.ValueType),
spans: &dataBlock{
offset: 10,
Review Comment:
The expected results were adjusted for reset-in-unmarshal behavior, but this
test still unmarshals into a nil destination slice. The bug fix is about
reusing a destination slice (dst[:0]) where underlying entries may still hold
stale tags/tagType maps. Add a subtest that pre-populates a dst slice (cap > 0)
with a blockMetadata containing non-empty tags/tagType, then call
unmarshalBlockMetadata(dst[:0], marshaled, tagType) and assert the decoded
result doesn’t contain the stale keys.
--
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]
