diff --git a/clients/azuredevopsrepo/branches.go b/clients/azuredevopsrepo/branches.go index cbef840f63f..784337aa3ba 100644 --- a/clients/azuredevopsrepo/branches.go +++ b/clients/azuredevopsrepo/branches.go @@ -20,6 +20,7 @@ import ( "sync" "github.com/microsoft/azure-devops-go-api/azuredevops/v7/git" + "github.com/ossf/scorecard/v5/clients" ) @@ -62,7 +63,6 @@ func (handler *branchesHandler) setup() error { handler.errSetup = nil }) return handler.errSetup - } func (handler *branchesHandler) getDefaultBranch() (*clients.BranchRef, error) { diff --git a/clients/azuredevopsrepo/branches_test.go b/clients/azuredevopsrepo/branches_test.go index b938e4f0feb..2bebd3dc09c 100644 --- a/clients/azuredevopsrepo/branches_test.go +++ b/clients/azuredevopsrepo/branches_test.go @@ -24,11 +24,12 @@ import ( ) func TestGetDefaultBranch(t *testing.T) { + t.Parallel() tests := []struct { - name string setupMock func() fnQueryBranch - expectedError bool + name string expectedName string + expectedError bool }{ { name: "successful branch retrieval", @@ -53,6 +54,7 @@ func TestGetDefaultBranch(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() handler := &branchesHandler{ queryBranch: tt.setupMock(), once: new(sync.Once), diff --git a/clients/azuredevopsrepo/client.go b/clients/azuredevopsrepo/client.go index 70ddec043c8..5c79bd09011 100644 --- a/clients/azuredevopsrepo/client.go +++ b/clients/azuredevopsrepo/client.go @@ -26,22 +26,24 @@ import ( "github.com/microsoft/azure-devops-go-api/azuredevops/v7" "github.com/microsoft/azure-devops-go-api/azuredevops/v7/git" + "github.com/ossf/scorecard/v5/clients" ) var ( - _ clients.RepoClient = &Client{} - errInputRepoType = errors.New("input repo should be of type azuredevopsrepo.Repo") + _ clients.RepoClient = &Client{} + errInputRepoType = errors.New("input repo should be of type azuredevopsrepo.Repo") + errDefaultBranchNotFound = errors.New("default branch not found") ) type Client struct { - repourl *Repo azdoClient git.Client - once sync.Once + ctx context.Context + repourl *Repo branches *branchesHandler commits *commitsHandler - ctx context.Context commitDepth int + once sync.Once } func (c *Client) InitRepo(inputRepo clients.Repo, commitSHA string, commitDepth int) error { @@ -135,7 +137,7 @@ func (c *Client) GetDefaultBranchName() (string, error) { return c.repourl.defaultBranch, nil } - return "", fmt.Errorf("%s", "default branch name is empty") + return "", fmt.Errorf("%w", errDefaultBranchNotFound) } func (c *Client) GetDefaultBranch() (*clients.BranchRef, error) { diff --git a/clients/azuredevopsrepo/client_test.go b/clients/azuredevopsrepo/client_test.go index 9773f695e3f..8f34e01c8f4 100644 --- a/clients/azuredevopsrepo/client_test.go +++ b/clients/azuredevopsrepo/client_test.go @@ -24,8 +24,8 @@ import ( type mockGitClient struct { git.Client - isDisabled bool err error + isDisabled bool } func (m *mockGitClient) GetRepository(ctx context.Context, args git.GetRepositoryArgs) (*git.GitRepository, error) { @@ -36,10 +36,11 @@ func (m *mockGitClient) GetRepository(ctx context.Context, args git.GetRepositor } func TestIsArchived(t *testing.T) { + t.Parallel() tests := []struct { + err error name string isDisabled bool - err error want bool wantErr bool }{ @@ -65,6 +66,7 @@ func TestIsArchived(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() client := &Client{ azdoClient: &mockGitClient{ isDisabled: tt.isDisabled, diff --git a/clients/azuredevopsrepo/commits.go b/clients/azuredevopsrepo/commits.go index 06fb9410877..abda5812007 100644 --- a/clients/azuredevopsrepo/commits.go +++ b/clients/azuredevopsrepo/commits.go @@ -16,24 +16,28 @@ package azuredevopsrepo import ( "context" + "errors" "fmt" "sync" "github.com/microsoft/azure-devops-go-api/azuredevops/v7/git" + "github.com/ossf/scorecard/v5/clients" ) +var errMultiplePullRequests = errors.New("expected 1 pull request for commit, got multiple") + type commitsHandler struct { gitClient git.Client ctx context.Context - once *sync.Once errSetup error + once *sync.Once repourl *Repo commitsRaw *[]git.GitCommitRef pullRequestsRaw *git.GitPullRequestQuery - commitDepth int getCommits fnGetCommits getPullRequestQuery fnGetPullRequestQuery + commitDepth int } func (handler *commitsHandler) init(ctx context.Context, repourl *Repo, commitDepth int) { @@ -81,8 +85,8 @@ func (handler *commitsHandler) setup() error { } commitIds := make([]string, len(*commits)) - for i, commit := range *commits { - commitIds[i] = *commit.CommitId + for i := range *commits { + commitIds[i] = *(*commits)[i].CommitId } pullRequestQuery := git.GetPullRequestQueryArgs{ @@ -117,7 +121,8 @@ func (handler *commitsHandler) listCommits() ([]clients.Commit, error) { } commits := make([]clients.Commit, len(*handler.commitsRaw)) - for i, commit := range *handler.commitsRaw { + for i := range *handler.commitsRaw { + commit := &(*handler.commitsRaw)[i] commits[i] = clients.Commit{ SHA: *commit.CommitId, Message: *commit.Comment, @@ -134,13 +139,14 @@ func (handler *commitsHandler) listCommits() ([]clients.Commit, error) { return nil, fmt.Errorf("error during commitsHandler.listPullRequests: %w", err) } - for i, commit := range commits { + for i := range commits { + commit := &commits[i] associatedPullRequest, ok := pullRequests[commit.SHA] if !ok { continue } - commits[i].AssociatedMergeRequest = associatedPullRequest + commit.AssociatedMergeRequest = associatedPullRequest } return commits, nil @@ -159,7 +165,7 @@ func (handler *commitsHandler) listPullRequests() (map[string]clients.PullReques } if len(azdoPullRequests) > 1 { - return nil, fmt.Errorf("expected 1 pull request for commit %s, got %d", commit, len(azdoPullRequests)) + return nil, errMultiplePullRequests } azdoPullRequest := azdoPullRequests[0] diff --git a/cmd/root.go b/cmd/root.go index bc50e7b8d4b..39d2881518d 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -221,18 +221,28 @@ func printCheckResults(enabledChecks checker.CheckNameToFnMap) { func makeRepo(uri string) (clients.Repo, error) { var repo clients.Repo var errGitHub, errGitLab, errAzureDevOps error - if repo, errGitHub = githubrepo.MakeGithubRepo(uri); errGitHub != nil { - if repo, errGitLab = gitlabrepo.MakeGitlabRepo(uri); errGitLab != nil { - _, experimental := os.LookupEnv("SCORECARD_EXPERIMENTAL") - if experimental { - repo, errAzureDevOps = azuredevopsrepo.MakeAzureDevOpsRepo(uri) - if errAzureDevOps != nil { - return nil, fmt.Errorf("unable to parse as github, gitlab, or azuredevops: %w", errors.Join(errGitHub, errGitLab, errAzureDevOps)) - } - } else { - return nil, fmt.Errorf("unable to parse as github or gitlab: %w", errors.Join(errGitHub, errGitLab)) - } + var compositeErr error + + repo, errGitHub = githubrepo.MakeGithubRepo(uri) + if errGitHub == nil { + return repo, nil + } + compositeErr = errors.Join(compositeErr, errGitHub) + + repo, errGitLab = gitlabrepo.MakeGitlabRepo(uri) + if errGitLab == nil { + return repo, nil + } + compositeErr = errors.Join(compositeErr, errGitLab) + + _, experimental := os.LookupEnv("SCORECARD_EXPERIMENTAL") + if experimental { + repo, errAzureDevOps = azuredevopsrepo.MakeAzureDevOpsRepo(uri) + if errAzureDevOps == nil { + return repo, nil } + compositeErr = errors.Join(compositeErr, errAzureDevOps) } - return repo, nil + + return nil, fmt.Errorf("unable to parse as github, gitlab, or azuredevops: %w", compositeErr) }