implement initial discover service #4665
Conversation
viambot
added
the
safe to test
go.mod
Outdated
| @@ -431,3 +431,5 @@ require ( | |||
| github.com/ziutek/mymysql v1.5.4 // indirect | |||
| golang.org/x/exp v0.0.0-20240904232852-e7e105dedf7e | |||
| ) | |||
|
|
|||
| replace go.viam.com/api => github.com/johnn193/api v0.0.0-20241231164642-99f059defc82 | |||
There was a problem hiding this comment.
will revert once api is merged
services/discovery/server_test.go
Outdated
| return discovery.NewRPCServiceServer(injectSvc).(pb.DiscoveryServiceServer), injectDiscovery, injectDiscovery2, nil | ||
| } | ||
|
|
||
| func TestDiscoveryDo(t *testing.T) { |
There was a problem hiding this comment.
since this is literally copy/paste from the generic service I should probably remove this test, but I'm also fine with leaving it in case we are worried this will somehow break
There was a problem hiding this comment.
It is good to test all APIs of a service to ensure that there is adequate coverage. I can't imagine it breaking - but say someone with no context joins viam in the future and removes the DoCommands accidentally - good to have all these tests fail :) . It does not have to be separate from your other test, you can add an injected DoCommand to the injected service in your test.
Good test to have with DoComamnd are: 1. test that it returns errors if no DoCommand is implemented with resource.Named, and 2. responds with the expected responses when a DoCommand is implemented.
There was a problem hiding this comment.
yeah I just copied the testing structure that other services had, which is why it was separate. Moved all tests into the same test function based on feedback.
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
services/discovery/server_test.go
Outdated
| // this was taken from proto_conversion_test.go | ||
| // | ||
| //nolint:thelper | ||
| func validateComponent(t *testing.T, actual, expected resource.Config) { |
There was a problem hiding this comment.
this is the same function as in proto_conversion_test.go
unsure if theres a good spot to put this function to use it in both spots, or if its fine to leave here
| } | ||
|
|
||
| // Service describes the functions that are available to the service. | ||
| type Service interface { |
There was a problem hiding this comment.
hmm, surprised this is called Service not discovery since the components follow. different pattern. But whatever, the other services do this so let's keep that format.
services/discovery/discovery.go
Outdated
| SubtypeName = "discovery" | ||
| ) | ||
|
|
||
| // API is a variable that identifies the slam resource API. |
There was a problem hiding this comment.
| // API is a variable that identifies the slam resource API. | |
| // API is a variable that identifies the discovery resource API. |
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
Initial implementation of the discover service in Go. Currently working on GetModelsFromModules in a separate branch to make it easier on reviewers
ticket: https://viam.atlassian.net/browse/RSDK-9620