chaokunyang commented on code in PR #3374: URL: https://github.com/apache/fory/pull/3374#discussion_r2895127492
########## go/fory/test_helper_test.go: ########## @@ -0,0 +1,79 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package fory + +import ( + "io" + "reflect" + "testing" +) + +// oneByteReader returns data byte by byte to ensure aggressively that all +// `fill()` boundaries, loops, and buffering conditions are tested. +type oneByteReader struct { + data []byte + pos int +} + +func (r *oneByteReader) Read(p []byte) (n int, err error) { + if r.pos >= len(r.data) { + return 0, io.EOF + } + if len(p) == 0 { + return 0, nil + } + p[0] = r.data[r.pos] + r.pos++ + return 1, nil +} + +// testDeserialize is a testing helper that performs standard in-memory +// deserialization and additionally wraps the payload in a slow one-byte +// stream reader to verify that stream decoding handles fragmented reads correctly. +func testDeserialize(t *testing.T, f *Fory, data []byte, v any) error { + t.Helper() + + // 1. First, deserialize from bytes (the fast path) + err := f.Deserialize(data, v) + if err != nil { + return err + } + + // 2. Deserialize from oneByteReader (the slow stream path) + // We create a new instance of the same target type to ensure clean state + vType := reflect.TypeOf(v) + if vType == nil || vType.Kind() != reflect.Ptr { + t.Fatalf("testDeserialize requires a pointer to a value, got %v", vType) + } + + stream := &oneByteReader{data: data, pos: 0} + + // Create a new stream reader. The stream context handles boundaries and compactions. + streamReader := NewInputStream(stream) + err = f.DeserializeFromStream(streamReader, v) Review Comment: This helper deserializes into the same target twice (first from bytes, then from stream). That can mask stream-path bugs because values populated by the first pass may still be present if the second pass does not fully overwrite fields. Please deserialize the stream path into a fresh value and compare results, so partial/incorrect stream decoding is detectable. ########## go/fory/stream_test.go: ########## @@ -0,0 +1,238 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package fory + +import ( + "bytes" + "io" + "testing" +) + +type StreamTestStruct struct { + ID int32 + Name string + Data []byte +} + +func TestStreamDeserialization(t *testing.T) { + f := New() + f.RegisterStruct(&StreamTestStruct{}, 100) + + original := &StreamTestStruct{ + ID: 42, + Name: "Stream Test", + Data: []byte{1, 2, 3, 4, 5}, + } + + data, err := f.Serialize(original) + if err != nil { + t.Fatalf("Serialize failed: %v", err) + } + + // 1. Test normal reader + reader := bytes.NewReader(data) + var decoded StreamTestStruct + err = f.DeserializeFromReader(reader, &decoded) + if err != nil { + t.Fatalf("DeserializeFromReader failed: %v", err) + } + + if decoded.ID != original.ID || decoded.Name != original.Name || !bytes.Equal(decoded.Data, original.Data) { + t.Errorf("Decoded value mismatch. Got: %+v, Want: %+v", decoded, original) + } +} + +// slowReader returns data byte by byte to test fill() logic and compaction +type slowReader struct { + data []byte + pos int +} + +func (r *slowReader) Read(p []byte) (n int, err error) { + if r.pos >= len(r.data) { + return 0, io.EOF + } + if len(p) == 0 { + return 0, nil + } + p[0] = r.data[r.pos] + r.pos++ + return 1, nil +} + +func TestStreamDeserializationSlow(t *testing.T) { + f := New() + f.RegisterStruct(&StreamTestStruct{}, 100) + + original := &StreamTestStruct{ + ID: 42, + Name: "Slow Stream Test with a reasonably long string and some data to trigger multiple fills", + Data: bytes.Repeat([]byte{0xAA}, 100), + } + + data, err := f.Serialize(original) + if err != nil { + t.Fatalf("Serialize failed: %v", err) + } + + // Test with slow reader and small minCap to force compaction/growth + reader := &slowReader{data: data} + var decoded StreamTestStruct + // Use small minCap (16) to force frequent fills and compactions + f.readCtx.buffer.ResetWithReader(reader, 16) Review Comment: This setup line does not affect the code path under test, because `DeserializeFromReader` immediately calls `ResetWithReader(r, 0)` internally and replaces the min-cap you set here. If the goal is to verify small-cap refill/compaction behavior, consider using `DeserializeFromStream` with `NewInputStreamWithMinCap(..., 16)` so the configured capacity is actually used. -- 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]
