Skip to content

Conversation

@Aaditya-ops-cpu
Copy link

This PR adds negative test cases to grpcdynamic to verify that InvokeRpc and InvokeRpcClientStream correctly return an error when provided with a message type that does not match the method descriptor and this matches the "needs more tests" note in the README regarding new V2 functionality. 👍😊

@jhump
Copy link
Owner

jhump commented Jan 9, 2026

@Aaditya-ops-cpu, thanks for the pull request, but these tests aren't particularly valuable. The "needs more tests" remark in the README is referring to the fact that new functionality in V2 doesn't yet have adequate test coverage. The grpcdynamic package is not new and has the same functionality as the V1 version of the package.

The main place remaining that still needs more test is the protoresolve package.

Also, if you want to contribute in the future, please run make ci locally to make sure everything looks good. That will save on iteration time. (For example, in this PR, there is a formatting issue that make ci could have caught before you pushed the changes to GitHub.)

@Aaditya-ops-cpu
Copy link
Author

Aaditya-ops-cpu commented Jan 9, 2026

@jhump Thanks for the clarification sir 👍. I misunderstood the README. I’ll take a look at adding tests around protoresolve instead and will make sure to run make ci locally for future contributions 👍.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants