Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

poc: rpc prop tests #1083

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

poc: rpc prop tests #1083

wants to merge 5 commits into from

Conversation

jharveyb
Copy link
Collaborator

Starts to address #1078 .

I think this logic can probably be simplified a fair amount wrt. reducing the # of switches. I think adding wrapper types with their own validity-checking functions would also help for that; handling proto enums makes the test a lot larger, but I think most of the REST endpoints will have enum fields to support arguments as strings.

Rapid has some functions to autogenerate values via reflection: https://pkg.go.dev/pgregory.net/rapid#Make

I'm not sure if that will populate the gRPC private fields like state, unknownFields, etc. IIUC we don't want those to be populated.

This small test found some missing checks, which would probably get rejected later on. Will peek at other RPCs with more fields to test out more complex invariants.

@coveralls
Copy link

coveralls commented Aug 14, 2024

Pull Request Test Coverage Report for Build 10426710362

Details

  • 19 of 65 (29.23%) changed or added relevant lines in 1 file are covered.
  • 34 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+0.08%) to 40.248%

Changes Missing Coverage Covered Lines Changed/Added Lines %
rpcserver.go 19 65 29.23%
Files with Coverage Reduction New Missed Lines %
commitment/tap.go 1 83.64%
tappsbt/create.go 2 53.85%
rpcserver.go 2 0.95%
asset/asset.go 2 81.54%
tapdb/universe.go 4 80.91%
tapgarden/caretaker.go 4 68.87%
tapdb/multiverse.go 7 60.32%
universe/interface.go 12 50.22%
Totals Coverage Status
Change from base Build 10425783374: 0.08%
Covered Lines: 23738
Relevant Lines: 58980

💛 - Coveralls

@jharveyb
Copy link
Collaborator Author

jharveyb commented Aug 19, 2024

I think this is in a decent state for now - this change to rapid made gRPC message generation much more ergonomic:

flyingmutant/rapid#72

I'd prefer to block merging this until that PR (or something similar enough) lands in a rapid release. We could pull in the fixes in a separate PR if needed.

The gRPC message generators could be moved elsewhere, but I imagine moving them to internal/test would cause an import cycle.

@Roasbeef Roasbeef modified the milestones: v0.4.2, v0.5 Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

4 participants