Included tests for internal/collector/github/factory

internal/collector/github/factory.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"strings"
 
 	"go.uber.org/zap"
 
@@ -29,6 +30,7 @@ import (
 type factory struct {
 	client *githubapi.Client
 	logger *zap.Logger
+	query  Query // this is used for testing purposes
 }
 
 func NewRepoFactory(client *githubapi.Client, logger *zap.Logger) projectrepo.Factory {
@@ -44,7 +46,11 @@ func (f *factory) New(ctx context.Context, u *url.URL) (projectrepo.Repo, error)
 		origURL: u,
 		logger:  f.logger.With(zap.String("url", u.String())),
 	}
-	if err := r.init(ctx); err != nil {
+	// this will be used for testing purposes
+	if f.query == nil {
+		f.query = NewQuery()
+	}
+	if err := r.init(ctx, f.query); err != nil {
 		if errors.Is(err, githubapi.ErrGraphQLNotFound) {
 			return nil, fmt.Errorf("%w: %s", projectrepo.ErrNoRepoFound, u)
 		} else {
@@ -55,5 +61,5 @@ func (f *factory) New(ctx context.Context, u *url.URL) (projectrepo.Repo, error)
 }
 
 func (f *factory) Match(u *url.URL) bool {
-	return u.Hostname() == "github.com"
+	return strings.ToLower(u.Hostname()) == "github.com"
 }

internal/collector/github/factory_test.go

@@ -0,0 +1,164 @@
+package github
+
+import (
+	"context"
+	"errors"
+	"net/url"
+	"reflect"
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"go.uber.org/zap"
+
+	"github.com/ossf/criticality_score/internal/collector/projectrepo"
+	"github.com/ossf/criticality_score/internal/githubapi"
+)
+
+func Test_factory_New(t *testing.T) {
+	type fields struct {
+		client *githubapi.Client
+		logger *zap.Logger
+	}
+	type args struct {
+		ctx context.Context
+		u   *url.URL
+	}
+	tests := []struct { //nolint:govet
+		name     string
+		fields   fields
+		args     args
+		repoData *BasicRepoData
+		queryErr error
+		want     projectrepo.Repo
+		wantErr  bool
+	}{
+		{
+			name: "r.Init() error is not equal to nil",
+			fields: fields{
+				client: &githubapi.Client{},
+				logger: zap.NewNop(),
+			},
+			args: args{
+				ctx: context.Background(),
+				u: &url.URL{
+					Host: "github.com",
+					Path: "/ossf/criticality_score",
+				},
+			},
+			queryErr: errors.New("query error"),
+			wantErr:  true,
+		},
+		{
+			name: "query error is ErrGraphQLNotFound",
+			fields: fields{
+				client: &githubapi.Client{},
+				logger: zap.NewNop(),
+			},
+			args: args{
+				ctx: context.Background(),
+				u: &url.URL{
+					Host: "github.com",
+					Path: "/ossf/criticality_score",
+				},
+			},
+			queryErr: githubapi.ErrGraphQLNotFound,
+			wantErr:  true,
+		},
+
+		// TODO: add test for r.Init not returning error
+		// Not able to do this right now because of legacy.FetchCreatedTime is not yet mocked.
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			f := NewRepoFactory(test.fields.client, test.fields.logger) // testing NewRepoFactory
+
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+			q := NewMockQuery(ctrl)
+			q.EXPECT().QueryBasicRepoData(gomock.Any(), gomock.Any(), gomock.Any()).Return(
+				test.repoData, test.queryErr).AnyTimes()
+
+			f = &factory{
+				client: f.(*factory).client,
+				logger: f.(*factory).logger,
+				query:  q,
+			}
+
+			got, err := f.New(test.args.ctx, test.args.u)
+			if (err != nil) != test.wantErr {
+				t.Errorf("New() error = %v, wantErr %v", err, test.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, test.want) {
+				t.Errorf("New() got = %v, want %v", got, test.want)
+			}
+		})
+	}
+}
+
+func Test_factory_Match(t *testing.T) {
+	type fields struct {
+		client *githubapi.Client
+		logger *zap.Logger
+	}
+	tests := []struct { //nolint:govet
+		name   string
+		fields fields
+		u      *url.URL
+		want   bool
+	}{
+		{
+			name: "valid",
+			fields: fields{
+				client: &githubapi.Client{},
+				logger: zap.NewNop(),
+			},
+			u: &url.URL{
+				Host: "github.com",
+			},
+			want: true,
+		},
+		{
+			name: "invalid",
+			fields: fields{
+				client: &githubapi.Client{},
+				logger: zap.NewNop(),
+			},
+			u: &url.URL{
+				Host: "invalid.com",
+			},
+			want: false,
+		},
+		{
+			name: "is a host name",
+			fields: fields{
+				client: &githubapi.Client{},
+				logger: zap.NewNop(),
+			},
+			u: &url.URL{
+				Host: "[github.com]:123",
+			},
+			want: true,
+		},
+		{
+			name: "uppercase",
+			fields: fields{
+				client: &githubapi.Client{},
+				logger: zap.NewNop(),
+			},
+			u: &url.URL{
+				Host: "GITHUB.COM",
+			},
+			want: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			f := NewRepoFactory(test.fields.client, test.fields.logger)
+
+			if got := f.Match(test.u); got != test.want {
+				t.Errorf("Match() = %v, want %v", got, test.want)
+			}
+		})
+	}
+}

internal/collector/github/queries.go

@@ -29,7 +29,7 @@ const (
 	legacyCommitLookback      = 365 * 24 * time.Hour
 )
 
-type basicRepoData struct {
+type BasicRepoData struct {
 	Name      string
 	URL       string
 	MirrorURL string
@@ -66,7 +66,7 @@ type basicRepoData struct {
 	} `graphql:"refs(refPrefix:\"refs/tags/\")"`
 }
 
-func queryBasicRepoData(ctx context.Context, client *githubv4.Client, u *url.URL) (*basicRepoData, error) {
+func (q query) QueryBasicRepoData(ctx context.Context, client *githubv4.Client, u *url.URL) (*BasicRepoData, error) {
 	// Search based on owner and repo name becaues the `repository` query
 	// better handles changes in ownership and repository name than the
 	// `resource` query.
@@ -75,7 +75,7 @@ func queryBasicRepoData(ctx context.Context, client *githubv4.Client, u *url.URL
 	owner := parts[0]
 	name := parts[1]
 	s := &struct {
-		Repository basicRepoData `graphql:"repository(owner: $repositoryOwner, name: $repositoryName)"`
+		Repository BasicRepoData `graphql:"repository(owner: $repositoryOwner, name: $repositoryName)"`
 	}{}
 	now := time.Now().UTC()
 	vars := map[string]any{
@@ -88,3 +88,9 @@ func queryBasicRepoData(ctx context.Context, client *githubv4.Client, u *url.URL
 	}
 	return &s.Repository, nil
 }
+
+type query struct{}
+
+func NewQuery() query {
+	return query{}
+}

internal/collector/github/query.go

@@ -0,0 +1,12 @@
+package github
+
+import (
+	"context"
+	"net/url"
+
+	"github.com/shurcooL/githubv4"
+)
+
+type Query interface {
+	QueryBasicRepoData(ctx context.Context, client *githubv4.Client, u *url.URL) (*BasicRepoData, error)
+}

internal/collector/github/repo.go

@@ -31,7 +31,7 @@ type repo struct {
 	origURL *url.URL
 	logger  *zap.Logger
 
-	BasicData *basicRepoData
+	BasicData *BasicRepoData
 	realURL   *url.URL
 	created   time.Time
 }
@@ -41,13 +41,9 @@ func (r *repo) URL() *url.URL {
 	return r.realURL
 }
 
-func (r *repo) init(ctx context.Context) error {
-	if r.BasicData != nil {
-		// Already finished. Don't init() more than once.
-		return nil
-	}
+func (r *repo) init(ctx context.Context, q Query) error {
 	r.logger.Debug("Fetching basic data from GitHub")
-	data, err := queryBasicRepoData(ctx, r.client.GraphQL(), r.origURL)
+	data, err := q.QueryBasicRepoData(ctx, r.client.GraphQL(), r.origURL)
 	if err != nil {
 		return err
 	}