From f853a9c34aba3feac1ad41847f3d4bb708f1b5a7 Mon Sep 17 00:00:00 2001 From: nathannaveen <42319948+nathannaveen@users.noreply.github.com> Date: Mon, 16 Jan 2023 16:33:55 -0600 Subject: [PATCH 1/2] Included tests for internal/collector/source - Included some tests for internal/collector/source Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com> --- internal/collector/depsdev/source.go | 2 +- internal/collector/depsdev/source_test.go | 184 ++++++++++++++++++++++ internal/mock/repo.go | 103 ++++++++++++ 3 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 internal/collector/depsdev/source_test.go create mode 100644 internal/mock/repo.go diff --git a/internal/collector/depsdev/source.go b/internal/collector/depsdev/source.go index 4b33d7e2..82fa745f 100644 --- a/internal/collector/depsdev/source.go +++ b/internal/collector/depsdev/source.go @@ -37,7 +37,7 @@ type depsDevSet struct { } func (s *depsDevSet) Namespace() signal.Namespace { - return signal.Namespace("depsdev") + return "depsdev" } type depsDevSource struct { diff --git a/internal/collector/depsdev/source_test.go b/internal/collector/depsdev/source_test.go new file mode 100644 index 00000000..1575f347 --- /dev/null +++ b/internal/collector/depsdev/source_test.go @@ -0,0 +1,184 @@ +package depsdev + +import ( + "context" + "net/url" + "reflect" + "testing" + "time" + + "github.com/golang/mock/gomock" + "go.uber.org/zap" + + "github.com/ossf/criticality_score/internal/collector/signal" + mocks "github.com/ossf/criticality_score/internal/mock" +) + +func Test_parseRepoURL(t *testing.T) { + tests := []struct { + name string + u *url.URL + wantProjectName string + wantProjectType string + }{ + { + name: "github.com", + u: &url.URL{ + Host: "github.com", + }, + wantProjectName: "", + wantProjectType: "GITHUB", + }, + { + name: "random", + u: &url.URL{ + Host: "random", + }, + wantProjectName: "", + wantProjectType: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotProjectName, gotProjectType := parseRepoURL(tt.u) + if gotProjectName != tt.wantProjectName { + t.Errorf("parseRepoURL() gotProjectName = %v, want %v", gotProjectName, tt.wantProjectName) + } + if gotProjectType != tt.wantProjectType { + t.Errorf("parseRepoURL() gotProjectType = %v, want %v", gotProjectType, tt.wantProjectType) + } + }) + } +} + +func Test_depsDevSource_Get(t *testing.T) { + type args struct { + ctx context.Context + rHostName string + jobID string + } + tests := []struct { //nolint:govet + name string + logger *zap.Logger + dependents *dependents + args args + want signal.Set + wantErr bool + }{ + { + name: "invalid url", + logger: zap.NewNop(), + dependents: &dependents{}, + args: args{ + ctx: context.Background(), + rHostName: "random", + }, + want: &depsDevSet{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + r := mocks.NewMockRepo(ctrl) + + r.EXPECT().URL().Return(&url.URL{Host: tt.args.rHostName}).AnyTimes() + + c := &depsDevSource{ + logger: tt.logger, + dependents: tt.dependents, + } + got, err := c.Get(tt.args.ctx, r, tt.args.jobID) + if (err != nil) != tt.wantErr { + t.Errorf("Get() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Get() got = %v, want %v", got, tt.want) + } + }) + } +} + +func TestNewSource(t *testing.T) { + type args struct { + ctx context.Context + logger *zap.Logger + projectID string + datasetName string + datasetTTL time.Duration + } + test := struct { //nolint:govet + name string + args args + want signal.Source + wantErr bool + }{ + name: "new client error", + args: args{ + ctx: context.Background(), + logger: zap.NewNop(), + projectID: "", + datasetName: "dataset", + datasetTTL: time.Hour, + }, + wantErr: true, + } + got, err := NewSource(test.args.ctx, test.args.logger, test.args.projectID, test.args.datasetName, test.args.datasetTTL) + if (err != nil) != test.wantErr { + t.Errorf("NewSource() error = %v, wantErr %v", err, test.wantErr) + return + } + if !reflect.DeepEqual(got, test.want) { + t.Errorf("NewSource() got = %v, want %v", got, test.want) + } +} + +func Test_depsDevSource_IsSupported(t *testing.T) { + type fields struct { + logger *zap.Logger + dependents *dependents + } + tests := []struct { + name string + fields fields + rHostName string + want bool + }{ + { + name: "github", + fields: fields{ + logger: zap.NewNop(), + }, + rHostName: "github.com", + want: true, + }, + { + name: "random", + fields: fields{ + logger: zap.NewNop(), + }, + rHostName: "random", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &depsDevSource{ + logger: tt.fields.logger, + dependents: tt.fields.dependents, + } + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + r := mocks.NewMockRepo(ctrl) + + r.EXPECT().URL().Return(&url.URL{Host: tt.rHostName}).AnyTimes() + + if got := c.IsSupported(r); got != tt.want { + t.Errorf("IsSupported() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/mock/repo.go b/internal/mock/repo.go new file mode 100644 index 00000000..fbecf8ed --- /dev/null +++ b/internal/mock/repo.go @@ -0,0 +1,103 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: /Users/nathannaveen/go/src/github.com/nathannaveen/criticality_score/internal/collector/projectrepo/repo.go + +// Package mocks is a generated GoMock package. +package mocks + +import ( + context "context" + url "net/url" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + projectrepo "github.com/ossf/criticality_score/internal/collector/projectrepo" +) + +// MockRepo is a mock of Repo interface. +type MockRepo struct { + ctrl *gomock.Controller + recorder *MockRepoMockRecorder +} + +// MockRepoMockRecorder is the mock recorder for MockRepo. +type MockRepoMockRecorder struct { + mock *MockRepo +} + +// NewMockRepo creates a new mock instance. +func NewMockRepo(ctrl *gomock.Controller) *MockRepo { + mock := &MockRepo{ctrl: ctrl} + mock.recorder = &MockRepoMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockRepo) EXPECT() *MockRepoMockRecorder { + return m.recorder +} + +// URL mocks base method. +func (m *MockRepo) URL() *url.URL { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "URL") + ret0, _ := ret[0].(*url.URL) + return ret0 +} + +// URL indicates an expected call of URL. +func (mr *MockRepoMockRecorder) URL() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "URL", reflect.TypeOf((*MockRepo)(nil).URL)) +} + +// MockFactory is a mock of Factory interface. +type MockFactory struct { + ctrl *gomock.Controller + recorder *MockFactoryMockRecorder +} + +// MockFactoryMockRecorder is the mock recorder for MockFactory. +type MockFactoryMockRecorder struct { + mock *MockFactory +} + +// NewMockFactory creates a new mock instance. +func NewMockFactory(ctrl *gomock.Controller) *MockFactory { + mock := &MockFactory{ctrl: ctrl} + mock.recorder = &MockFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { + return m.recorder +} + +// Match mocks base method. +func (m *MockFactory) Match(arg0 *url.URL) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Match", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// Match indicates an expected call of Match. +func (mr *MockFactoryMockRecorder) Match(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Match", reflect.TypeOf((*MockFactory)(nil).Match), arg0) +} + +// New mocks base method. +func (m *MockFactory) New(arg0 context.Context, arg1 *url.URL) (projectrepo.Repo, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "New", arg0, arg1) + ret0, _ := ret[0].(projectrepo.Repo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// New indicates an expected call of New. +func (mr *MockFactoryMockRecorder) New(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockFactory)(nil).New), arg0, arg1) +} From 7518e88a137e330b542f9cb5ed8c2763d19f9557 Mon Sep 17 00:00:00 2001 From: nathannaveen <42319948+nathannaveen@users.noreply.github.com> Date: Wed, 1 Mar 2023 18:20:51 -0600 Subject: [PATCH 2/2] Updated based on code review Signed-off-by: nathannaveen <42319948+nathannaveen@users.noreply.github.com> --- internal/collector/depsdev/source_test.go | 92 +++++++++---------- internal/mock/repo.go | 103 ---------------------- 2 files changed, 44 insertions(+), 151 deletions(-) delete mode 100644 internal/mock/repo.go diff --git a/internal/collector/depsdev/source_test.go b/internal/collector/depsdev/source_test.go index 1575f347..f5df8ad9 100644 --- a/internal/collector/depsdev/source_test.go +++ b/internal/collector/depsdev/source_test.go @@ -11,9 +11,17 @@ import ( "go.uber.org/zap" "github.com/ossf/criticality_score/internal/collector/signal" - mocks "github.com/ossf/criticality_score/internal/mock" ) +type mockRepo string + +func (r mockRepo) URL() *url.URL { + u := &url.URL{ + Host: string(r), + } + return u +} + func Test_parseRepoURL(t *testing.T) { tests := []struct { name string @@ -38,14 +46,14 @@ func Test_parseRepoURL(t *testing.T) { wantProjectType: "", }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotProjectName, gotProjectType := parseRepoURL(tt.u) - if gotProjectName != tt.wantProjectName { - t.Errorf("parseRepoURL() gotProjectName = %v, want %v", gotProjectName, tt.wantProjectName) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gotProjectName, gotProjectType := parseRepoURL(test.u) + if gotProjectName != test.wantProjectName { + t.Errorf("parseRepoURL() gotProjectName = %v, want %v", gotProjectName, test.wantProjectName) } - if gotProjectType != tt.wantProjectType { - t.Errorf("parseRepoURL() gotProjectType = %v, want %v", gotProjectType, tt.wantProjectType) + if gotProjectType != test.wantProjectType { + t.Errorf("parseRepoURL() gotProjectType = %v, want %v", gotProjectType, test.wantProjectType) } }) } @@ -57,7 +65,7 @@ func Test_depsDevSource_Get(t *testing.T) { rHostName string jobID string } - tests := []struct { //nolint:govet + test := struct { //nolint:govet name string logger *zap.Logger dependents *dependents @@ -65,39 +73,30 @@ func Test_depsDevSource_Get(t *testing.T) { want signal.Set wantErr bool }{ - { - name: "invalid url", - logger: zap.NewNop(), - dependents: &dependents{}, - args: args{ - ctx: context.Background(), - rHostName: "random", - }, - want: &depsDevSet{}, + name: "invalid url", + logger: zap.NewNop(), + dependents: &dependents{}, + args: args{ + ctx: context.Background(), + rHostName: "random", }, + want: &depsDevSet{}, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - r := mocks.NewMockRepo(ctrl) - r.EXPECT().URL().Return(&url.URL{Host: tt.args.rHostName}).AnyTimes() + ctrl := gomock.NewController(t) + defer ctrl.Finish() - c := &depsDevSource{ - logger: tt.logger, - dependents: tt.dependents, - } - got, err := c.Get(tt.args.ctx, r, tt.args.jobID) - if (err != nil) != tt.wantErr { - t.Errorf("Get() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("Get() got = %v, want %v", got, tt.want) - } - }) + c := &depsDevSource{ + logger: test.logger, + dependents: test.dependents, + } + got, err := c.Get(test.args.ctx, mockRepo(test.args.rHostName), test.args.jobID) + if (err != nil) != test.wantErr { + t.Errorf("Get() error = %v, wantErr %v", err, test.wantErr) + return + } + if !reflect.DeepEqual(got, test.want) { + t.Errorf("Get() got = %v, want %v", got, test.want) } } @@ -125,6 +124,7 @@ func TestNewSource(t *testing.T) { }, wantErr: true, } + got, err := NewSource(test.args.ctx, test.args.logger, test.args.projectID, test.args.datasetName, test.args.datasetTTL) if (err != nil) != test.wantErr { t.Errorf("NewSource() error = %v, wantErr %v", err, test.wantErr) @@ -163,21 +163,17 @@ func Test_depsDevSource_IsSupported(t *testing.T) { want: false, }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { c := &depsDevSource{ - logger: tt.fields.logger, - dependents: tt.fields.dependents, + logger: test.fields.logger, + dependents: test.fields.dependents, } ctrl := gomock.NewController(t) defer ctrl.Finish() - r := mocks.NewMockRepo(ctrl) - - r.EXPECT().URL().Return(&url.URL{Host: tt.rHostName}).AnyTimes() - - if got := c.IsSupported(r); got != tt.want { - t.Errorf("IsSupported() = %v, want %v", got, tt.want) + if got := c.IsSupported(mockRepo(test.rHostName)); got != test.want { + t.Errorf("IsSupported() = %v, want %v", got, test.want) } }) } diff --git a/internal/mock/repo.go b/internal/mock/repo.go deleted file mode 100644 index fbecf8ed..00000000 --- a/internal/mock/repo.go +++ /dev/null @@ -1,103 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: /Users/nathannaveen/go/src/github.com/nathannaveen/criticality_score/internal/collector/projectrepo/repo.go - -// Package mocks is a generated GoMock package. -package mocks - -import ( - context "context" - url "net/url" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - projectrepo "github.com/ossf/criticality_score/internal/collector/projectrepo" -) - -// MockRepo is a mock of Repo interface. -type MockRepo struct { - ctrl *gomock.Controller - recorder *MockRepoMockRecorder -} - -// MockRepoMockRecorder is the mock recorder for MockRepo. -type MockRepoMockRecorder struct { - mock *MockRepo -} - -// NewMockRepo creates a new mock instance. -func NewMockRepo(ctrl *gomock.Controller) *MockRepo { - mock := &MockRepo{ctrl: ctrl} - mock.recorder = &MockRepoMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockRepo) EXPECT() *MockRepoMockRecorder { - return m.recorder -} - -// URL mocks base method. -func (m *MockRepo) URL() *url.URL { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "URL") - ret0, _ := ret[0].(*url.URL) - return ret0 -} - -// URL indicates an expected call of URL. -func (mr *MockRepoMockRecorder) URL() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "URL", reflect.TypeOf((*MockRepo)(nil).URL)) -} - -// MockFactory is a mock of Factory interface. -type MockFactory struct { - ctrl *gomock.Controller - recorder *MockFactoryMockRecorder -} - -// MockFactoryMockRecorder is the mock recorder for MockFactory. -type MockFactoryMockRecorder struct { - mock *MockFactory -} - -// NewMockFactory creates a new mock instance. -func NewMockFactory(ctrl *gomock.Controller) *MockFactory { - mock := &MockFactory{ctrl: ctrl} - mock.recorder = &MockFactoryMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { - return m.recorder -} - -// Match mocks base method. -func (m *MockFactory) Match(arg0 *url.URL) bool { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Match", arg0) - ret0, _ := ret[0].(bool) - return ret0 -} - -// Match indicates an expected call of Match. -func (mr *MockFactoryMockRecorder) Match(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Match", reflect.TypeOf((*MockFactory)(nil).Match), arg0) -} - -// New mocks base method. -func (m *MockFactory) New(arg0 context.Context, arg1 *url.URL) (projectrepo.Repo, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "New", arg0, arg1) - ret0, _ := ret[0].(projectrepo.Repo) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// New indicates an expected call of New. -func (mr *MockFactoryMockRecorder) New(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockFactory)(nil).New), arg0, arg1) -}