Simplify and update code gen tests #14
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Motivation: The protobuf code generator doesn't actually do a lot: it parses a protobuf file descriptor and turns that into a request for the code generator provided by the GRPCCodeGen module. This means that the tests should focus on the conversion rather than the generated code (although this is still useful to test at a high level). As it stands all tests are done on the generated code rather than the conversion to a code generator request. The existing tests also hand roll a protobuf file descriptor which is tedious and error prone. The upstream code generator also has changes which aren't yet reflected here. Modifications: - Define some test proto files for which we generate serialized protobuf file descriptor sets. - Add a suite of code gen parser tests which check the request created from the file descriptor proto. - Add a single code generator test. - Downgrade an access level from public to package as public wasn't necessary. Result: Better tests
glbrntt
added
🆕 semver/minor
rnro
reviewed
Dec 3, 2024
| @@ -86,14 +92,16 @@ internal struct ProtobufCodeGenParser { | |||
| } | |||
|
|
|||
| extension ProtobufCodeGenParser { | |||
| fileprivate var codeDependencies: [Dependency] { | |||
| fileprivate func codeDependencies( | |||
There was a problem hiding this comment.
nit: are the newlines in the signature intentional/needed?
There was a problem hiding this comment.
No, I prefer a single line where possible. It was probably just an artefact of how I edited the existing code.
rnro
reviewed
Dec 3, 2024
There was a problem hiding this comment.
It might be worth considering if there's value in a CI job to make sure these don't fall out of step with the generated code.
There was a problem hiding this comment.
Good point, I'll do this in a follow up given it's been auto-merged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation:
The protobuf code generator doesn't actually do a lot: it parses a protobuf file descriptor and turns that into a request for the code generator provided by the GRPCCodeGen module. This means that the tests should focus on the conversion rather than the generated code (although this is still useful to test at a high level).
As it stands all tests are done on the generated code rather than the conversion to a code generator request. The existing tests also hand roll a protobuf file descriptor which is tedious and error prone.
The upstream code generator also has changes which aren't yet reflected here.
Modifications:
Result:
Better tests