Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
internal/impl: fix TestMarshalMessageSetLazyRace (was a no-op!)
This test was accidentally migrated from proto.GetExtension to proto.HasExtension in a semi-automated change and nobody noticed. proto.HasExtension does not actually trigger lazy extension decoding. I verified this using the dlv debugger: % go test -tags protolegacy -c -gcflags "all=-N -l" % dlv exec ./impl.test Type 'help' for list of commands. (dlv) r -test.run=TestMarshalMessageSetLazyRace -test.v Process restarted with PID 3586699 (dlv) b TestMarshalMessageSetLazyRace Breakpoint 1 set at 0xac9bf6 for google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500 (dlv) c === RUN TestMarshalMessageSetLazyRace > [Breakpoint 1] google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500 (hits goroutine(32):1 total:1) (PC: 0xac9bf6) => 500: func TestMarshalMessageSetLazyRace(t *testing.T) { (dlv) b ExtensionField.lazyInit Breakpoint 2 set at 0x8da076 for google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (dlv) c --- PASS: TestMarshalMessageSetLazyRace (4.07s) PASS Process 3586699 has exited with status 0 (dlv) exit Note how breakpoint 2 is unexpectedly not hit! Here is how the output changes with my fix: % go test -tags protolegacy -c -gcflags "all=-N -l" % dlv --check-go-version=false exec ./impl.test Type 'help' for list of commands. (dlv) r -test.run=TestMarshalMessageSetLazyRace -test.v Process restarted with PID 3587344 (dlv) b TestMarshalMessageSetLazyRace Breakpoint 1 set at 0xac9bf6 for google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500 (dlv) c === RUN TestMarshalMessageSetLazyRace > [Breakpoint 1] google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace() ./lazy_test.go:500 (hits goroutine(28):1 total:1) (PC: 0xac9bf6) => 500: func TestMarshalMessageSetLazyRace(t *testing.T) { (dlv) b ExtensionField.lazyInit Breakpoint 2 set at 0x8da076 for google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (dlv) c > [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(68):1 total:4) (PC: 0x8da076) > [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(70):1 total:4) (PC: 0x8da076) > [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(58):1 total:4) (PC: 0x8da076) > [Breakpoint 2] google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit() ./codec_extension.go:123 (hits goroutine(54):1 total:4) (PC: 0x8da076) => 123: func (f *ExtensionField) lazyInit() { (dlv) bt 0 0x00000000008da076 in google.golang.org/protobuf/internal/impl.(*ExtensionField).lazyInit at ./codec_extension.go:123 1 0x00000000008dac3e in google.golang.org/protobuf/internal/impl.(*ExtensionField).Value at ./codec_extension.go:180 2 0x000000000094e5d9 in google.golang.org/protobuf/internal/impl.(*extensionMap).Get at ./message_reflect.go:276 3 0x000000000095a2ee in google.golang.org/protobuf/internal/impl.(*messageState).Get at ./message_reflect_gen.go:90 4 0x0000000000890099 in google.golang.org/protobuf/proto.GetExtension at /usr/local/google/home/stapelberg/protobuf/proto/extension.go:90 5 0x0000000000aca7d4 in google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace.func2.1 at ./lazy_test.go:545 6 0x0000000000aca625 in google.golang.org/protobuf/internal/impl_test.TestMarshalMessageSetLazyRace.func2 at ./lazy_test.go:550 7 0x00000000006001c1 in runtime.goexit at /usr/lib/google-golang/src/runtime/asm_amd64.s:1700 Change-Id: Ie7a8621064af412a1db7efb3ad6b8473f9db58e8 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/624416 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Chressie Himpel <chressie@google.com>
- Loading branch information