3 ms·

I hate this API and Go's handling of protocol buffers in general. Especially preparing test data for it makes for some of the most cumbersome and unwieldy files

by kubb 2y ago

I hate this API and Go's handling of protocol buffers in general. Especially preparing test data for it makes for some of the most cumbersome and unwieldy files that you will ever come across. Combined with table driven testing you have thousands upon thousands of lines of data with an unbelievably long identifiers that can't be inferred (e.g. in array literals) that is usually copy pasted around and slightly changed. Updating and understanding all of that is a nightmare and if you miss a coma or a brace somewhere, the compiler isn't smart enough to point you to where so you get lines upon lines of syntax errors. But, being opaque has some advantages for sure.

alienchow 2y ago
The testing practice I've seen is to have a testdata/ directory with a bunch of textprotos for different test cases. If you're using Bazel, just include the entire directory glob as data dependency for the unit tests. The test tables are essentially just appropriately named textproto filenames that are unmarshaled into the proto message to be tested. Then again I've also seen people do these thousand line in-code literal string protos which really grind my gears.
atombender 2y ago
The generated Go code situation has always been wild to me. For example, every message embeds protoimpl.MessageState and a bunch of other types, which contain mutexes. That means proto structs cannot be copied or compared byte-for-byte like normal Go structs can. For several years I used the GoGo Protobuf SDK. It was vastly superior to the awful Javaesque Go code that the official compiler generated. It allowed structs to be pure data structs, was much more performant, and supported a bunch of options to generate native-feeling, ergonomic Go code. But the Google team refused to partake in any such improvements, and GoGo was shut down as the burden of following the upstream implementation became too big. [1] I'm not an expert, but as far as I understand, the extra struct junk is mostly to avoid having a parallel set of types for metadata (including reflection). It's unclear to me why these can't simply be generated as internal types with some nice API on top. Clearly the new field metadata adds to this extra information, and the Go team is moving in the opposite direction of what I thought the future was — they're doubling down on stuffing metadata into the structs, and making the structs bigger and even less wieldy. I understand how this might make things more performant, but I was hoping this sort of thing could be solved with the type system, especially now that we have generics. For example, surely lazy field access could be done like this: type Info struct { User LazyProto[User] } userName := user.Get().Name [1] https://x.com/awalterschulze/status/1584553056100057088 https://x.com/awalterschulze/status/1584553056100057088
throwaway894345 2y ago
I haven't used protocol buffers, but in general any kind of code generation produces awful code. I much prefer generating the machine spec (protocol buffers, in this case) from Go code rather than the other way around. It's not a perfect solution, but it's much better than dealing with generated code in my experience.
- MobiusHorizons 2y ago
  Generated code tends to look very formulaic, but it doesn’t have to be unreadable. As a primative it is incredibly powerful, and can be easier to maintain than alternatives. You definitely need good build tooling though. Ideally you won’t need to look at the generated code and can infer the interface from the input file.
  - throwaway894345 2y ago
    It's pretty hard to generate formulaic Go code while ensuring that, for example, methods don't conflict with exported member names, or in cases when you're making identifiers from multiple words, that the identifiers for `FooBar Baz` does not conflict with the identifier for `Foo BarBaz` since both of these would naturally be rendered in Go as `FooBarBaz`. Or even how you model an optional type: in Go, an idiomatic optional may use nil for reference types, -1 for nonnegative integers, an empty string for nonempty strings, or a `(T, bool)` tuple. You can definitely make a code generator that does a decent job at modeling all of these things, but I've never seen one--they usually try to pick a rule that works for all cases, like `Foo_BarBaz` or using an extra indirection for optionals, which means all cases are not-idiomatic.
GeneralMayhem 2y ago
I find that the best way to set up test cases, regardless of language, is usually to use string constants in the proto text format (https://protobuf.dev/reference/protobuf/textformat-spec/ https://protobuf.dev/reference/protobuf/textformat-spec/). For arrays, and especially for oneofs, it's way less verbose than how things are represented in Go, C++, or Java, and generally at least on par with the Python constructors. Maps are the only thing that suffer a bit, because they're represented as a list of key-val pairs (like in the wire format) instead of an actual map. Your language's compiler won't help you debug, but the parse-text-proto function can point to the source of the issue on a line/character level. With Go generics - and equivalent in most other languages - you can write a 5-line helper function that takes a string in that format and either returns a valid proto value (using the generic type param to decide which type to unmarshal) or `t.Fatal()`s. You would never do this in production code, but as a way to represent hand-written proto values it's pretty hard to beat.
- kubb 2y ago
  Unless someone with authority in your workplace makes a rule against doing that…
  - GeneralMayhem 2y ago
    If your problem is humans making arbitrary and nonsensical decisions about how you can do your job, then you have a non-technical problem, and it's unlikely that any technical solution will solve it.
    - kubb 2y ago
      Fair. Best not to code.
lalaithion 2y ago
We put test inputs and outputs in testdata/test_name.in.textpb and testdata/test_name.out.textpb, respectively. Way nicer than defining both your inputs and your desired outputs in go code, even compared to not using protobuf at all, to the point where we occasionally write some proto definitions just for test inputs and outputs.

by kubb 2y ago

alienchow 2y ago

The testing practice I've seen is to have a testdata/ directory with a bunch of textprotos for different test cases. If you're using Bazel, just include the entire directory glob as data dependency for the unit tests. The test tables are essentially just appropriately named textproto filenames that are unmarshaled into the proto message to be tested. Then again I've also seen people do these thousand line in-code literal string protos which really grind my gears.

atombender 2y ago

The generated Go code situation has always been wild to me. For example, every message embeds protoimpl.MessageState and a bunch of other types, which contain mutexes. That means proto structs cannot be copied or compared byte-for-byte like normal Go structs can. For several years I used the GoGo Protobuf SDK. It was vastly superior to the awful Javaesque Go code that the official compiler generated. It allowed structs to be pure data structs, was much more performant, and supported a bunch of options to generate native-feeling, ergonomic Go code. But the Google team refused to partake in any such improvements, and GoGo was shut down as the burden of following the upstream implementation became too big. [1] I'm not an expert, but as far as I understand, the extra struct junk is mostly to avoid having a parallel set of types for metadata (including reflection). It's unclear to me why these can't simply be generated as internal types with some nice API on top. Clearly the new field metadata adds to this extra information, and the Go team is moving in the opposite direction of what I thought the future was — they're doubling down on stuffing metadata into the structs, and making the structs bigger and even less wieldy. I understand how this might make things more performant, but I was hoping this sort of thing could be solved with the type system, especially now that we have generics. For example, surely lazy field access could be done like this: type Info struct { User LazyProto[User] } userName := user.Get().Name [1] https://x.com/awalterschulze/status/1584553056100057088 https://x.com/awalterschulze/status/1584553056100057088

throwaway894345 2y ago

I haven't used protocol buffers, but in general any kind of code generation produces awful code. I much prefer generating the machine spec (protocol buffers, in this case) from Go code rather than the other way around. It's not a perfect solution, but it's much better than dealing with generated code in my experience.

MobiusHorizons 2y ago
Generated code tends to look very formulaic, but it doesn’t have to be unreadable. As a primative it is incredibly powerful, and can be easier to maintain than alternatives. You definitely need good build tooling though. Ideally you won’t need to look at the generated code and can infer the interface from the input file.
- throwaway894345 2y ago
  It's pretty hard to generate formulaic Go code while ensuring that, for example, methods don't conflict with exported member names, or in cases when you're making identifiers from multiple words, that the identifiers for `FooBar Baz` does not conflict with the identifier for `Foo BarBaz` since both of these would naturally be rendered in Go as `FooBarBaz`. Or even how you model an optional type: in Go, an idiomatic optional may use nil for reference types, -1 for nonnegative integers, an empty string for nonempty strings, or a `(T, bool)` tuple. You can definitely make a code generator that does a decent job at modeling all of these things, but I've never seen one--they usually try to pick a rule that works for all cases, like `Foo_BarBaz` or using an extra indirection for optionals, which means all cases are not-idiomatic.

GeneralMayhem 2y ago

I find that the best way to set up test cases, regardless of language, is usually to use string constants in the proto text format (https://protobuf.dev/reference/protobuf/textformat-spec/ https://protobuf.dev/reference/protobuf/textformat-spec/). For arrays, and especially for oneofs, it's way less verbose than how things are represented in Go, C++, or Java, and generally at least on par with the Python constructors. Maps are the only thing that suffer a bit, because they're represented as a list of key-val pairs (like in the wire format) instead of an actual map. Your language's compiler won't help you debug, but the parse-text-proto function can point to the source of the issue on a line/character level. With Go generics - and equivalent in most other languages - you can write a 5-line helper function that takes a string in that format and either returns a valid proto value (using the generic type param to decide which type to unmarshal) or `t.Fatal()`s. You would never do this in production code, but as a way to represent hand-written proto values it's pretty hard to beat.

kubb 2y ago
Unless someone with authority in your workplace makes a rule against doing that…
- GeneralMayhem 2y ago
  If your problem is humans making arbitrary and nonsensical decisions about how you can do your job, then you have a non-technical problem, and it's unlikely that any technical solution will solve it.
  - kubb 2y ago
    Fair. Best not to code.

lalaithion 2y ago

We put test inputs and outputs in testdata/test_name.in.textpb and testdata/test_name.out.textpb, respectively. Way nicer than defining both your inputs and your desired outputs in go code, even compared to not using protobuf at all, to the point where we occasionally write some proto definitions just for test inputs and outputs.