Mocking framework for unit test

[prietyc123]

/kind user-story

User Story

As a dev/QE I want to write unit test for the function which has dependency on packages, registry, interfaces etc.

Acceptance Criteria

• We should set up the mocking framework to ease the writing of unit test

Links

• Basic framework doc : https://docs.google.com/document/d/1wbLpxF37p82oAl4u8R7gLy-MHtYMK6lkIB8Rr7d7rCs/edit?usp=sharing , However we still need to add the examples so that team can identify their comfortability.

/kind user-story

[girishramnani]

we need to have interfaces accross the code to allowing mocking. so we need a pre-req issue over it

[girishramnani]

@prietyc123 as part of research can you please pick up an interface from the code and mock it with both the framework to see what makes sense

[prietyc123]

I have looked into the odo code and found the interface at one place within the devfile https://github.com/openshift/odo/blob/master/pkg/devfile/adapters/common/interface.go and tried mocking it with mockgen but unfortunately the interface has nested interface in it which is not supported within mocking frameworks. However luckily I was looking into the test file and got the example of gomock we are using in the code base. As we are already using gomock in our code base IMO its better to implement gomock structure without changing the already existing tests. Also Gomock provide better APIs and assertion facility than testify.

Example:
Tests which has gomock examples in odo https://github.com/openshift/odo/blob/master/pkg/devfile/adapters/docker/component/adapter_test.go#L370-L485 , we are also looking for similar structure within our code base. It defines a better understanding of the Gomock framework architecture and process. As I didn't find any other interface implementation in our code I would suggest let's design a better architecture for refactoring the code with client and context interface implementation, use decoupling approach ... I mean whoever is refactoring the code should have it in their mind that we need interface for each and every packages. It seems a long run plan to me for writing more and more unit tests.

Once we are done with refactoring and get the interface I will start implementing the unit test with one of the dev team member, I think mrinal would be best person for this because he has worked a lot on this. @kadel @girishramnani Please let me know your suggestions on this. Also @mik-dass please add your point too if I missed something.

[girishramnani]

use https://github.com/openshift/odo/blob/a28e37838c5cbaa396f6675fe90a9986168165f5/pkg/envinfo/envinfo.go and https://github.com/openshift/odo/blob/master/pkg/devfile/adapters/common/interface.go
to finalise a mocking framework

[prietyc123]

Two different mocking structure with interface https://github.com/openshift/odo/blob/a28e37838c5cbaa396f6675fe90a9986168165f5/pkg/envinfo/envinfo.go#L90-L97

Gomock:
Generated the mock interface using mockgen
$ mockgen -source=$pwd/pkg/envinfo/envinfo.go -destination=$pwd/pkg/envinfo/foo.go -package=envinfo LocalConfigProvider

// Code generated by MockGen. DO NOT EDIT.
// Source: ./pkg/envinfo/envinfo.go

// Package envinfo is a generated GoMock package.
package envinfo

import (
	gomock "github.com/golang/mock/gomock"
	reflect "reflect"
)

// MockLocalConfigProvider is a mock of LocalConfigProvider interface
type MockLocalConfigProvider struct {
	ctrl     *gomock.Controller
	recorder *MockLocalConfigProviderMockRecorder
}

// MockLocalConfigProviderMockRecorder is the mock recorder for MockLocalConfigProvider
type MockLocalConfigProviderMockRecorder struct {
	mock *MockLocalConfigProvider
}

// NewMockLocalConfigProvider creates a new mock instance
func NewMockLocalConfigProvider(ctrl *gomock.Controller) *MockLocalConfigProvider {
	mock := &MockLocalConfigProvider{ctrl: ctrl}
	mock.recorder = &MockLocalConfigProviderMockRecorder{mock}
	return mock
}

// EXPECT returns an object that allows the caller to indicate expected use
func (m *MockLocalConfigProvider) EXPECT() *MockLocalConfigProviderMockRecorder {
	return m.recorder
}

// GetApplication mocks base method
func (m *MockLocalConfigProvider) GetApplication() string {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "GetApplication")
	ret0, _ := ret[0].(string)
	return ret0
}

// GetApplication indicates an expected call of GetApplication
func (mr *MockLocalConfigProviderMockRecorder) GetApplication() *gomock.Call {
	mr.mock.ctrl.T.Helper()
	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetApplication", reflect.TypeOf((*MockLocalConfigProvider)(nil).GetApplication))
}

// GetName mocks base method
func (m *MockLocalConfigProvider) GetName() string {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "GetName")
	ret0, _ := ret[0].(string)
	return ret0
}

// GetName indicates an expected call of GetName
func (mr *MockLocalConfigProviderMockRecorder) GetName() *gomock.Call {
	mr.mock.ctrl.T.Helper()
	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetName", reflect.TypeOf((*MockLocalConfigProvider)(nil).GetName))
}

// GetNamespace mocks base method
func (m *MockLocalConfigProvider) GetNamespace() string {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "GetNamespace")
	ret0, _ := ret[0].(string)
	return ret0
}

// GetNamespace indicates an expected call of GetNamespace
func (mr *MockLocalConfigProviderMockRecorder) GetNamespace() *gomock.Call {
	mr.mock.ctrl.T.Helper()
	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetNamespace", reflect.TypeOf((*MockLocalConfigProvider)(nil).GetNamespace))
}

// GetDebugPort mocks base method
func (m *MockLocalConfigProvider) GetDebugPort() int {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "GetDebugPort")
	ret0, _ := ret[0].(int)
	return ret0
}

// GetDebugPort indicates an expected call of GetDebugPort
func (mr *MockLocalConfigProviderMockRecorder) GetDebugPort() *gomock.Call {
	mr.mock.ctrl.T.Helper()
	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDebugPort", reflect.TypeOf((*MockLocalConfigProvider)(nil).GetDebugPort))
}

// GetURL mocks base method
func (m *MockLocalConfigProvider) GetURL() []EnvInfoURL {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "GetURL")
	ret0, _ := ret[0].([]EnvInfoURL)
	return ret0
}

// GetURL indicates an expected call of GetURL
func (mr *MockLocalConfigProviderMockRecorder) GetURL() *gomock.Call {
	mr.mock.ctrl.T.Helper()
	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetURL", reflect.TypeOf((*MockLocalConfigProvider)(nil).GetURL))
}

// Exists mocks base method
func (m *MockLocalConfigProvider) Exists() bool {
	m.ctrl.T.Helper()
	ret := m.ctrl.Call(m, "Exists")
	ret0, _ := ret[0].(bool)
	return ret0
}

// Exists indicates an expected call of Exists
func (mr *MockLocalConfigProviderMockRecorder) Exists() *gomock.Call {
	mr.mock.ctrl.T.Helper()
	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Exists", reflect.TypeOf((*MockLocalConfigProvider)(nil).Exists))
}

Testify:
Generated the mock interface using mockery
$ mockery -dir=./pkg/envinfo/ -name LocalConfigProvider -output ./pkg/envinfo/mocks/

// Code generated by mockery v1.0.0. DO NOT EDIT.

package mocks

import (
	envinfo "github.com/openshift/odo/pkg/envinfo"
	mock "github.com/stretchr/testify/mock"
)

// LocalConfigProvider is an autogenerated mock type for the LocalConfigProvider type
type LocalConfigProvider struct {
	mock.Mock
}

// Exists provides a mock function with given fields:
func (_m *LocalConfigProvider) Exists() bool {
	ret := _m.Called()

	var r0 bool
	if rf, ok := ret.Get(0).(func() bool); ok {
		r0 = rf()
	} else {
		r0 = ret.Get(0).(bool)
	}

	return r0
}

// GetApplication provides a mock function with given fields:
func (_m *LocalConfigProvider) GetApplication() string {
	ret := _m.Called()

	var r0 string
	if rf, ok := ret.Get(0).(func() string); ok {
		r0 = rf()
	} else {
		r0 = ret.Get(0).(string)
	}

	return r0
}

// GetDebugPort provides a mock function with given fields:
func (_m *LocalConfigProvider) GetDebugPort() int {
	ret := _m.Called()

	var r0 int
	if rf, ok := ret.Get(0).(func() int); ok {
		r0 = rf()
	} else {
		r0 = ret.Get(0).(int)
	}

	return r0
}

// GetName provides a mock function with given fields:
func (_m *LocalConfigProvider) GetName() string {
	ret := _m.Called()

	var r0 string
	if rf, ok := ret.Get(0).(func() string); ok {
		r0 = rf()
	} else {
		r0 = ret.Get(0).(string)
	}

	return r0
}

// GetNamespace provides a mock function with given fields:
func (_m *LocalConfigProvider) GetNamespace() string {
	ret := _m.Called()

	var r0 string
	if rf, ok := ret.Get(0).(func() string); ok {
		r0 = rf()
	} else {
		r0 = ret.Get(0).(string)
	}

	return r0
}

// GetURL provides a mock function with given fields:
func (_m *LocalConfigProvider) GetURL() []envinfo.EnvInfoURL {
	ret := _m.Called()

	var r0 []envinfo.EnvInfoURL
	if rf, ok := ret.Get(0).(func() []envinfo.EnvInfoURL); ok {
		r0 = rf()
	} else {
		if ret.Get(0) != nil {
			r0 = ret.Get(0).([]envinfo.EnvInfoURL)
		}
	}

	return r0
}

I am still working on the unit test part and will try to update the tests asap. For now we can compare both the mocking framework structure of the same interface.

[girishramnani]

We need to convert this into an epic and add track all the testing refactor under this

[openshift-bot]

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

[prietyc123]

Closing this issue via #4110 and extending the refactoring part in #4421

/close

[openshift-ci-robot]

@prietyc123: Closing this issue.

In response to this:

Closing this issue via #4110 and extending the refactoring part in #4421

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.