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

Decoupling and testing techniques #796

Open
AaronH88 opened this issue Jun 13, 2023 · 0 comments
Open

Decoupling and testing techniques #796

AaronH88 opened this issue Jun 13, 2023 · 0 comments
Labels
type:enhancement New feature or request

Comments

@AaronH88
Copy link
Contributor

AaronH88 commented Jun 13, 2023

As golang does not have the keyword "implements", interfaces are resolved at run time. This opens golang up to the principle "Accept Interfaces, return Structs" which is a very powerful technique for decoupling and as a result, creates injection points for mocking.

I propose refactoring receptor using this technique. Here is what it could look like:

  • Each package will be decoupled from each other via a local package interface
  • Package interfaces could follow a naming convention outlining the relationship they represent
  • Fields outside the calling package will no longer be directly accessible, but instead, must be retrieved via Getters
  • The scope of this technique should not be limited to inter-package relationships, but should also be expanded to include 3rd party libraries and local functions/structs/objects.

Here is an example of what this would look like using the "Workceptor" and "Netceptor" relationship. Currently within the "Workceptor" struct their is a "Netceptor" field. I propose replacing this with an interface like so:

type WorkceptorsNetceptor interface {
	NodeID() string
	AddWorkCommand(typeName string, verifySignature bool) error
	GetClientTLSConfig(name string, expectedHostName string, expectedHostNameType netceptor.ExpectedHostnameType) (*tls.Config, error)
	GetLogger() *logger.ReceptorLogger
	DialContext(ctx context.Context, node string, service string, tlscfg *tls.Config) (*netceptor.Conn, error)
}

// Workceptor is the main object that handles unit-of-work management.
type Workceptor struct {
	ctx               context.Context
	Cancel            context.CancelFunc
	nc                WorkceptorsNetceptor
	dataDir           string
	workTypesLock     *sync.RWMutex
	workTypes         map[string]*workType
	activeUnitsLock   *sync.RWMutex
	activeUnits       map[string]WorkUnit
	SigningKey        string
	SigningExpiration time.Duration
	VerifyingKey      string
}

Why do this?

Decoupling and injection.
Lets take testing as an example, currently their is no way to expose "Workceptor" as a unit because it is tightly coupled to "Netceptor". This means that any test or "Workceptor" functions are automatically integration tests, they will always require "Netceptor" objects to be set up and return correct values.

So if we were to follow the above suggestion, this interface will give us an injection point in which we can mock out all "Netceptor" interactions. Couple this with the mockgen tool and we can easily create and control the responses from "Netceptor" in our unit tests. Here is what that could look like:

// set up mocks
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockNetceptor := mock_workceptor.NewMockWorkceptorsNetceptor(ctrl)
logger := logger.NewReceptorLogger("")
// During workceptor setup, netceptor functions are called
mockNetceptor.EXPECT().GetLogger().AnyTimes().Return(logger)
mockNetceptor.EXPECT().NodeID().Return("test")

// set up workceptor using the mocked netceptor
w, err := workceptor.New(ctx, mockNetceptor, "/tmp")
if err != nil {
	t.Errorf("Error while creating Workceptor: %v", err)
}

// test workceptors "Register Worker" function
mockNetceptor.EXPECT().AddWorkCommand(gomock.Any(), gomock.Any()).Return(nil)
w.RegisterWorker("testType", workFunc, false)

As you can see from above, using the "EXPECT()" will allow us to force passing and failing responses, allowing us to control a "unit" of work for testing.

All feedback welcome. Let me know your thoughts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants