Skip to content

Commit

Permalink
internal/conformance: make conformance tests work with editions
Browse files Browse the repository at this point in the history
In scope of this change, I had to fix the `IsPacked()` implementation
for field descriptors because it was not returning the correct values
for edition protos.

Change-Id: Ic1ba9d0b3552ddf16360a80336c14632f2ce6f16
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/566039
Auto-Submit: Lasse Folger <[email protected]>
Reviewed-by: Michael Stapelberg <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
  • Loading branch information
lfolger authored and gopherbot committed Feb 26, 2024
1 parent 055c812 commit 416d517
Show file tree
Hide file tree
Showing 6 changed files with 6,929 additions and 7 deletions.
2 changes: 2 additions & 0 deletions internal/cmd/generate-protos/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ func generateRemoteProtos() {
{"", "conformance/conformance.proto", "google.golang.org/protobuf/internal/testprotos/conformance;conformance"},
{"src", "google/protobuf/test_messages_proto2.proto", "google.golang.org/protobuf/internal/testprotos/conformance;conformance"},
{"src", "google/protobuf/test_messages_proto3.proto", "google.golang.org/protobuf/internal/testprotos/conformance;conformance"},
{"src", "google/protobuf/editions/golden/test_messages_proto2_editions.proto", "google.golang.org/protobuf/internal/testprotos/conformance/editions;editions"},
{"src", "google/protobuf/editions/golden/test_messages_proto3_editions.proto", "google.golang.org/protobuf/internal/testprotos/conformance/editions;editions"},

// Benchmark protos.
// TODO: The protobuf repo no longer includes benchmarks.
Expand Down
11 changes: 10 additions & 1 deletion internal/conformance/conformance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"google.golang.org/protobuf/proto"

pb "google.golang.org/protobuf/internal/testprotos/conformance"
epb "google.golang.org/protobuf/internal/testprotos/conformance/editions"
)

func init() {
Expand All @@ -45,6 +46,7 @@ func Test(t *testing.T) {
"--failure_list", "failing_tests.txt",
"--text_format_failure_list", "failing_tests_text_format.txt",
"--enforce_recommended",
"--maximum_edition", "2023",
os.Args[0])
cmd.Env = append(os.Environ(), "RUN_AS_CONFORMANCE_PLUGIN=1")
out, err := cmd.CombinedOutput()
Expand Down Expand Up @@ -95,8 +97,15 @@ func main() {

func handle(req *pb.ConformanceRequest) (res *pb.ConformanceResponse) {
var msg proto.Message = &pb.TestAllTypesProto2{}
if req.GetMessageType() == "protobuf_test_messages.proto3.TestAllTypesProto3" {
switch req.GetMessageType() {
case "protobuf_test_messages.proto3.TestAllTypesProto3":
msg = &pb.TestAllTypesProto3{}
case "protobuf_test_messages.proto2.TestAllTypesProto2":
msg = &pb.TestAllTypesProto2{}
case "protobuf_test_messages.editions.proto3.TestAllTypesProto3":
msg = &epb.TestAllTypesProto3{}
case "protobuf_test_messages.editions.proto2.TestAllTypesProto2":
msg = &epb.TestAllTypesProto2{}
}

// Unmarshal the test message.
Expand Down
5 changes: 5 additions & 0 deletions internal/conformance/failing_tests_text_format.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
Recommended.Editions_Proto3.ProtobufInput.MessageUnknownFields_Print.TextFormatOutput
Recommended.Editions_Proto3.TextFormatInput.StringLiteralShortUnicodeEscapeSurrogatePairBytes
Recommended.Editions_Proto3.TextFormatInput.StringLiteralShortUnicodeEscapeSurrogatePairString
Recommended.Editions_Proto3.TextFormatInput.StringLiteralUnicodeEscapeSurrogatePairLongShortBytes
Recommended.Editions_Proto3.TextFormatInput.StringLiteralUnicodeEscapeSurrogatePairLongShortString
Recommended.Proto3.ProtobufInput.MessageUnknownFields_Print.TextFormatOutput
Recommended.Proto3.TextFormatInput.StringLiteralShortUnicodeEscapeSurrogatePairBytes
Recommended.Proto3.TextFormatInput.StringLiteralShortUnicodeEscapeSurrogatePairString
Expand Down
19 changes: 13 additions & 6 deletions internal/filedesc/desc.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,19 @@ func (fd *Field) HasOptionalKeyword() bool {
return (fd.L0.ParentFile.L1.Syntax == protoreflect.Proto2 && fd.L1.Cardinality == protoreflect.Optional && fd.L1.ContainingOneof == nil) || fd.L1.IsProto3Optional
}
func (fd *Field) IsPacked() bool {
if !fd.L1.HasPacked && fd.L0.ParentFile.L1.Syntax != protoreflect.Proto2 && fd.L1.Cardinality == protoreflect.Repeated {
switch fd.L1.Kind {
case protoreflect.StringKind, protoreflect.BytesKind, protoreflect.MessageKind, protoreflect.GroupKind:
default:
return true
}
if fd.L1.Cardinality != protoreflect.Repeated {
return false
}
switch fd.L1.Kind {
case protoreflect.StringKind, protoreflect.BytesKind, protoreflect.MessageKind, protoreflect.GroupKind:
return false
}
if fd.L0.ParentFile.L1.Syntax == protoreflect.Editions {
return fd.L1.EditionFeatures.IsPacked
}
if fd.L0.ParentFile.L1.Syntax == protoreflect.Proto3 {
// proto3 repeated fields are packed by default.
return !fd.L1.HasPacked || fd.L1.IsPacked
}
return fd.L1.IsPacked
}
Expand Down
Loading

0 comments on commit 416d517

Please sign in to comment.