🌱 Refactor to make it testable

- Related to #1568 Signed-off-by: naveensrinivasan <[email protected]>
ossf · Mar 11, 2022 · 1995bc3 · 1995bc3
1 parent f2a132a
commit 1995bc3
Show file tree

Hide file tree

Showing 7 changed files with 832 additions and 32 deletions.
diff --git a/Makefile b/Makefile
@@ -110,7 +110,7 @@ cron/data/metadata.pb.go: cron/data/metadata.proto |  $(PROTOC)
 	protoc --go_out=../../../ cron/data/metadata.proto
 
 generate-mocks: ## Compiles and generates all mocks using mockgen.
-generate-mocks: clients/mockclients/repo_client.go clients/mockclients/repo.go clients/mockclients/cii_client.go checks/mockclients/vulnerabilities.go
+generate-mocks: clients/mockclients/repo_client.go clients/mockclients/repo.go clients/mockclients/cii_client.go checks/mockclients/vulnerabilities.go checks/mockclients/packagemanager.go
 clients/mockclients/repo_client.go: clients/repo_client.go
 	# Generating MockRepoClient
 	$(MOCKGEN) -source=clients/repo_client.go -destination=clients/mockclients/repo_client.go -package=mockrepo -copyright_file=clients/mockclients/license.txt
@@ -123,7 +123,9 @@ clients/mockclients/cii_client.go: clients/cii_client.go
 checks/mockclients/vulnerabilities.go: clients/vulnerabilities.go
 	# Generating MockCIIClient
 	$(MOCKGEN) -source=clients/vulnerabilities.go -destination=clients/mockclients/vulnerabilities.go -package=mockrepo -copyright_file=clients/mockclients/license.txt
-
+checks/mockclients/packagemanager.go: cmd/packagemanager_client.go
+	# Generating MockPackageManagerClient
+	$(MOCKGEN) -source=cmd/packagemanager_client.go -destination=clients/mockclients/packagemanager.go -package=mockrepo -copyright_file=clients/mockclients/license.txt
 generate-docs: ## Generates docs
 generate-docs: validate-docs docs/checks.md
 docs/checks.md: docs/checks/internal/checks.yaml docs/checks/internal/*.go docs/checks/internal/generate/*.go

diff --git a/clients/mockclients/packagemanager.go b/clients/mockclients/packagemanager.go
diff --git a/clients/repo_client.go b/clients/repo_client.go
@@ -15,7 +15,9 @@
 // Package clients defines the interface for RepoClient and related structs.
 package clients
 
-import "errors"
+import (
+	"errors"
+)
 
 // ErrUnsupportedFeature indicates an API that is not supported by the client.
 var ErrUnsupportedFeature = errors.New("unsupported feature")

diff --git a/cmd/package_managers.go b/cmd/package_managers.go
@@ -18,8 +18,6 @@ package cmd
 import (
 	"encoding/json"
 	"fmt"
-	"net/http"
-	"time"
 
 	sce "github.com/ossf/scorecard/v4/errors"
 )
@@ -29,24 +27,24 @@ type packageMangerResponse struct {
 	exists         bool
 }
 
-// TODO(#1568): Add unit tests for this.
-func fetchGitRepositoryFromPackageManagers(npm, pypi, rubygems string) (packageMangerResponse, error) {
+func fetchGitRepositoryFromPackageManagers(npm, pypi, rubygems string,
+	manager packageManagerClient) (packageMangerResponse, error) {
 	if npm != "" {
-		gitRepo, err := fetchGitRepositoryFromNPM(npm)
+		gitRepo, err := fetchGitRepositoryFromNPM(npm, manager)
 		return packageMangerResponse{
 			exists:         true,
 			associatedRepo: gitRepo,
 		}, err
 	}
 	if pypi != "" {
-		gitRepo, err := fetchGitRepositoryFromPYPI(pypi)
+		gitRepo, err := fetchGitRepositoryFromPYPI(pypi, manager)
 		return packageMangerResponse{
 			exists:         true,
 			associatedRepo: gitRepo,
 		}, err
 	}
 	if rubygems != "" {
-		gitRepo, err := fetchGitRepositoryFromRubyGems(rubygems)
+		gitRepo, err := fetchGitRepositoryFromRubyGems(rubygems, manager)
 		return packageMangerResponse{
 			exists:         true,
 			associatedRepo: gitRepo,
@@ -79,14 +77,9 @@ type rubyGemsSearchResults struct {
 }
 
 // Gets the GitHub repository URL for the npm package.
-// nolint: noctx
-func fetchGitRepositoryFromNPM(packageName string) (string, error) {
+func fetchGitRepositoryFromNPM(packageName string, packageManager packageManagerClient) (string, error) {
 	npmSearchURL := "https://registry.npmjs.org/-/v1/search?text=%s&size=1"
-	const timeout = 10
-	client := &http.Client{
-		Timeout: timeout * time.Second,
-	}
-	resp, err := client.Get(fmt.Sprintf(npmSearchURL, packageName))
+	resp, err := packageManager.Get(npmSearchURL, packageName)
 	if err != nil {
 		return "", sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("failed to get npm package json: %v", err))
 	}
@@ -105,14 +98,9 @@ func fetchGitRepositoryFromNPM(packageName string) (string, error) {
 }
 
 // Gets the GitHub repository URL for the pypi package.
-// nolint: noctx
-func fetchGitRepositoryFromPYPI(packageName string) (string, error) {
+func fetchGitRepositoryFromPYPI(packageName string, manager packageManagerClient) (string, error) {
 	pypiSearchURL := "https://pypi.org/pypi/%s/json"
-	const timeout = 10
-	client := &http.Client{
-		Timeout: timeout * time.Second,
-	}
-	resp, err := client.Get(fmt.Sprintf(pypiSearchURL, packageName))
+	resp, err := manager.Get(pypiSearchURL, packageName)
 	if err != nil {
 		return "", sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("failed to get pypi package json: %v", err))
 	}
@@ -131,14 +119,9 @@ func fetchGitRepositoryFromPYPI(packageName string) (string, error) {
 }
 
 // Gets the GitHub repository URL for the rubygems package.
-// nolint: noctx
-func fetchGitRepositoryFromRubyGems(packageName string) (string, error) {
+func fetchGitRepositoryFromRubyGems(packageName string, manager packageManagerClient) (string, error) {
 	rubyGemsSearchURL := "https://rubygems.org/api/v1/gems/%s.json"
-	const timeout = 10
-	client := &http.Client{
-		Timeout: timeout * time.Second,
-	}
-	resp, err := client.Get(fmt.Sprintf(rubyGemsSearchURL, packageName))
+	resp, err := manager.Get(rubyGemsSearchURL, packageName)
 	if err != nil {
 		return "", sce.WithMessage(sce.ErrScorecardInternal, fmt.Sprintf("failed to get ruby gem json: %v", err))
 	}