Skip to content
This repository has been archived by the owner on Jan 19, 2023. It is now read-only.

ClientState uses a private struct #2305

Closed
wwitzel3 opened this issue Apr 12, 2021 · 4 comments · Fixed by #2325
Closed

ClientState uses a private struct #2305

wwitzel3 opened this issue Apr 12, 2021 · 4 comments · Fixed by #2325
Assignees
Labels
bug Something isn't working

Comments

@wwitzel3
Copy link
Contributor

wwitzel3 commented Apr 12, 2021

ClientState is a struct out of internal and makes it non-mockable for testing.

Use an interface to expose this to plugin authors from the service package.

type ClientState interface {
	ClientID() string
	Filters() []Filter
	Namespace() string
	ContextName() string
}

Slack Thread: https://vmware.slack.com/archives/CESL848DN/p1618251260122500

@wwitzel3 wwitzel3 added the bug Something isn't working label Apr 12, 2021
@wwitzel3
Copy link
Contributor Author

@xtreme-vikram-yadav you mind taking this one?

@wwitzel3
Copy link
Contributor Author

Something like

var _ service.ClientState = (*clientState)(nil)

type clientState struct {
	clientID    string          `json:"clientID"`
	filters     []octant.Filter `json:"filters"`
	namespace   string          `json:"namespace"`
	contextName string          `json:"contextName"`
}

func (c *clientState) ClientID() string {
	return c.clientID
}
func (c *clientState) Filters() []octant.Filter {
	return c.filters
}
func (c *clientState) Namespace() string {
	return c.namespace
}
func (c *clientState) ContextName() string {
	return c.contextName
}

@wwitzel3
Copy link
Contributor Author

And then updating tests / imports to deal with any circular deps that shake out and also need to be lifted up.

@xtreme-vikram-yadav
Copy link
Contributor

Sure. Picking this up.

xtreme-vikram-yadav pushed a commit to xtreme-vikram-yadav/octant that referenced this issue Apr 13, 2021
xtreme-vikram-yadav pushed a commit to xtreme-vikram-yadav/octant that referenced this issue Apr 14, 2021
xtreme-vikram-yadav pushed a commit to xtreme-vikram-yadav/octant that referenced this issue Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants