Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 66 additions & 14 deletions pkg/devops/getazure/getazure.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,15 @@ func getRepoAnalyse(params ParamsProjectAzure, gitClient git.Client) ([]ProjectB
// Skip this repository if SingleBranch is set but not found
continue
}
largestRepoBranch = *repo.DefaultBranch
branchName, ok := defaultBranchName(&repo)
if !ok {
// Branch analysis failed and the repo has no usable default
// branch (e.g. disabled/archived). Skip it instead of
// dereferencing a nil pointer and crashing the whole run.
loggers.Errorf("\r❌ Skipping repo %s/%s: branch analysis failed and no default branch is set: %v", *project.Name, *repo.Name, err)
continue
}
largestRepoBranch = branchName
} else {
// Check if SingleBranch is set and the returned branch is not SingleBranch
if params.SingleBranch != "" && !params.DefaultB && largestRepoBranch != params.SingleBranch {
Expand Down Expand Up @@ -581,10 +589,21 @@ func analyzeRepoBranches(parms ParamsProjectAzure, projectKey string, repo strin
return largestRepoBranch, nbrbranch, brsize, nil

}
func getMostImportantBranch(ctx context.Context, gitClient git.Client, projectID string, repoID string, periode int, DefaultB bool, Singlebranch string) (string, int64, int, error) {
// defaultBranchName safely extracts a repository's default branch name, with
// the "refs/heads/" prefix stripped. The Azure DevOps API marks DefaultBranch
// as omitempty, so GetRepository can legitimately return it as nil — for
// example for a disabled/archived repository, or one whose HEAD ref was never
// initialized. Dereferencing it unconditionally previously crashed the whole
// analysis with "panic: runtime error: invalid memory address or nil pointer
// dereference". The bool result reports whether a usable default branch exists.
func defaultBranchName(repo *git.GitRepository) (string, bool) {
if repo == nil || repo.DefaultBranch == nil || *repo.DefaultBranch == "" {
return "", false
}
return strings.TrimPrefix(*repo.DefaultBranch, REF), true
}

var defaultBranch string
var err error
func getMostImportantBranch(ctx context.Context, gitClient git.Client, projectID string, repoID string, periode int, DefaultB bool, Singlebranch string) (string, int64, int, error) {

since := time.Now().AddDate(0, periode, 0)
sinceStr := since.Format(time.RFC3339)
Expand All @@ -597,15 +616,22 @@ func getMostImportantBranch(ctx context.Context, gitClient git.Client, projectID
if err != nil {
return "", 0, 0, err
}
defaultBranch = *repo.DefaultBranch

// Prioritize DefaultB over Singlebranch if DefaultB is true
if DefaultB {
return handleDefaultOrSingleBranch(ctx, gitClient, projectID, repoID, strings.TrimPrefix(defaultBranch, REF), "", sinceStr)
} else if Singlebranch != "" {
branchName, hasDefault := defaultBranchName(repo)

switch {
case DefaultB && hasDefault:
// Default-branch mode with a known default branch: analyze it directly.
return handleDefaultOrSingleBranch(ctx, gitClient, projectID, repoID, branchName, "", sinceStr)
case Singlebranch != "":
return handleDefaultOrSingleBranch(ctx, gitClient, projectID, repoID, "", Singlebranch, sinceStr)
} else {
return handleNonDefaultBranch(ctx, gitClient, projectID, repoID, sinceStr, defaultBranch)
default:
// Either we are not in default-branch mode, or the repository has no
// default branch set (e.g. a disabled/archived repo, or one whose HEAD
// ref was never initialized). Instead of skipping it, scan every branch
// and pick the most active one. branchName is the fallback default and
// may be empty when none is set.
return handleNonDefaultBranch(ctx, gitClient, projectID, repoID, sinceStr, branchName)
}
}
func handleNonDefaultBranch(ctx context.Context, gitClient git.Client, projectID string, repoID string, sinceStr string, defaultBranch string) (string, int64, int, error) {
Expand All @@ -622,21 +648,44 @@ func handleNonDefaultBranch(ctx context.Context, gitClient git.Client, projectID
return "", 0, 0, err
}

// firstBranch is the ultimate fallback: the first branch with a usable name.
// It keeps the repository in the analysis when no branch has commits in the
// window and the repo has no default branch set.
var firstBranch string
for _, branch := range *branches {
if branch.Name == nil {
continue
}
name := strings.TrimPrefix(*branch.Name, REF)
if firstBranch == "" {
firstBranch = name
}

commitCount, branchCommitSize, err := getCommitDetails(ctx, gitClient, projectID, repoID, *branch.Name, sinceStr)
if err != nil {
return "", 0, 0, err
}

if commitCount > maxCommits {
maxCommits = commitCount
mostImportantBranch = strings.TrimPrefix(*branch.Name, REF)
mostImportantBranch = name
totalCommitSize = branchCommitSize
}
}

if maxCommits == 0 {
mostImportantBranch = strings.TrimPrefix(defaultBranch, REF)
// No branch had commits in the analysis window. Prefer the repo's
// default branch when known, otherwise fall back to the first branch so
// the repository is still analyzed rather than dropped.
if trimmed := strings.TrimPrefix(defaultBranch, REF); trimmed != "" {
mostImportantBranch = trimmed
} else {
mostImportantBranch = firstBranch
}
}

if mostImportantBranch == "" {
return "", 0, 0, fmt.Errorf("repository %s has no analyzable branch", repoID)
}

return mostImportantBranch, totalCommitSize, len(*branches), nil
Expand Down Expand Up @@ -683,7 +732,10 @@ func handleDefaultOrSingleBranch(ctx context.Context, gitClient git.Client, proj
if err != nil {
return "", 0, 0, err
}
commitSize = int64(*repo.Size)
// Size is an omitempty pointer and may be nil for some repositories.
if repo.Size != nil {
commitSize = int64(*repo.Size)
}
}

return branchName, commitSize, 1, nil
Expand Down
251 changes: 251 additions & 0 deletions pkg/devops/getazure/getazure_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,262 @@
package getazure

import (
"context"
"io"
"testing"
"time"

"github.com/SonarSource-Demos/sonar-golc/pkg/utils"
"github.com/briandowns/spinner"
"github.com/google/uuid"
"github.com/microsoft/azure-devops-go-api/azuredevops/core"
"github.com/microsoft/azure-devops-go-api/azuredevops/git"
)

func strPtr(s string) *string { return &s }

// fakeGitClient embeds git.Client (so it satisfies the 121-method interface)
// and only overrides the calls exercised by the branch-resolution and
// repo-analysis logic (GetRepository, GetRepositories, GetItems, GetBranches,
// GetCommits). Any un-overridden method is nil and will panic if called — which
// is the signal that a test reached code it should not.
type fakeGitClient struct {
git.Client
repo *git.GitRepository
repoErr error
repos *[]git.GitRepository
items *[]git.GitItem
branches *[]git.GitBranchStats
branchErr error
commits map[string]int // branch name -> commit count in window
}

Comment thread
gitar-bot[bot] marked this conversation as resolved.
func (f *fakeGitClient) GetRepository(_ context.Context, _ git.GetRepositoryArgs) (*git.GitRepository, error) {
return f.repo, f.repoErr
}
func (f *fakeGitClient) GetRepositories(_ context.Context, _ git.GetRepositoriesArgs) (*[]git.GitRepository, error) {
return f.repos, nil
}
func (f *fakeGitClient) GetItems(_ context.Context, _ git.GetItemsArgs) (*[]git.GitItem, error) {
return f.items, nil
}
func (f *fakeGitClient) GetBranches(_ context.Context, _ git.GetBranchesArgs) (*[]git.GitBranchStats, error) {
return f.branches, f.branchErr
}
func (f *fakeGitClient) GetCommits(_ context.Context, a git.GetCommitsArgs) (*[]git.GitCommitRef, error) {
v := ""
if a.SearchCriteria != nil && a.SearchCriteria.ItemVersion != nil && a.SearchCriteria.ItemVersion.Version != nil {
v = *a.SearchCriteria.ItemVersion.Version
}
out := make([]git.GitCommitRef, f.commits[v])
return &out, nil
}

func branchStats(names ...*string) *[]git.GitBranchStats {
out := make([]git.GitBranchStats, 0, len(names))
for _, n := range names {
out = append(out, git.GitBranchStats{Name: n})
}
return &out
}

func TestHandleNonDefaultBranch(t *testing.T) {
tests := []struct {
name string
branches *[]git.GitBranchStats
commits map[string]int
defaultB string
wantBranch string
wantNbr int
wantErr bool
}{
{
name: "picks branch with most commits",
branches: branchStats(strPtr("main"), strPtr("develop")),
commits: map[string]int{"main": 2, "develop": 5},
wantBranch: "develop", wantNbr: 2,
},
{
name: "no commits, no default -> first branch",
branches: branchStats(strPtr("main"), strPtr("develop")),
commits: map[string]int{},
wantBranch: "main", wantNbr: 2,
},
{
name: "no commits, default set -> default branch",
branches: branchStats(strPtr("main"), strPtr("develop")),
commits: map[string]int{},
defaultB: "refs/heads/develop",
wantBranch: "develop", wantNbr: 2,
},
{
name: "nil branch name skipped, picks next valid",
branches: branchStats(nil, strPtr("release")),
commits: map[string]int{"release": 3},
wantBranch: "release", wantNbr: 2,
},
{
name: "no analyzable branch -> error",
branches: branchStats(nil),
commits: map[string]int{},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fc := &fakeGitClient{branches: tt.branches, commits: tt.commits}
branch, _, nbr, err := handleNonDefaultBranch(context.Background(), fc, "proj", "repo", "2020-01-01T00:00:00Z", tt.defaultB)
if tt.wantErr {
if err == nil {
t.Fatalf("expected error, got branch=%q", branch)
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if branch != tt.wantBranch {
t.Errorf("branch = %q, want %q", branch, tt.wantBranch)
}
if nbr != tt.wantNbr {
t.Errorf("nbrbranch = %d, want %d", nbr, tt.wantNbr)
}
})
}
}

func TestGetMostImportantBranch(t *testing.T) {
t.Run("default-branch mode with default present uses it", func(t *testing.T) {
fc := &fakeGitClient{
repo: &git.GitRepository{DefaultBranch: strPtr("refs/heads/main")},
branches: branchStats(strPtr("main")),
commits: map[string]int{"main": 3},
}
branch, _, nbr, err := getMostImportantBranch(context.Background(), fc, "proj", "repo", -1, true, "")
if err != nil || branch != "main" || nbr != 1 {
t.Fatalf("got (%q, %d, %v), want (main, 1, nil)", branch, nbr, err)
}
})

t.Run("default-branch mode with nil default falls back to biggest branch", func(t *testing.T) {
fc := &fakeGitClient{
repo: &git.GitRepository{DefaultBranch: nil}, // the bug trigger
branches: branchStats(strPtr("main"), strPtr("develop")),
commits: map[string]int{"main": 2, "develop": 5},
}
branch, _, nbr, err := getMostImportantBranch(context.Background(), fc, "proj", "repo", -1, true, "")
if err != nil || branch != "develop" || nbr != 2 {
t.Fatalf("got (%q, %d, %v), want (develop, 2, nil)", branch, nbr, err)
}
})

t.Run("single-branch mode resolves the named branch", func(t *testing.T) {
fc := &fakeGitClient{
repo: &git.GitRepository{DefaultBranch: strPtr("refs/heads/main")},
branches: branchStats(strPtr("main"), strPtr("feature")),
commits: map[string]int{"feature": 1},
}
branch, _, nbr, err := getMostImportantBranch(context.Background(), fc, "proj", "repo", -1, false, "feature")
if err != nil || branch != "feature" || nbr != 1 {
t.Fatalf("got (%q, %d, %v), want (feature, 1, nil)", branch, nbr, err)
}
})

t.Run("GetRepository error is propagated", func(t *testing.T) {
fc := &fakeGitClient{repoErr: context.DeadlineExceeded}
if _, _, _, err := getMostImportantBranch(context.Background(), fc, "proj", "repo", -1, true, ""); err == nil {
t.Fatal("expected error to propagate")
}
})
}

func TestHandleDefaultOrSingleBranch_NilSize(t *testing.T) {
// commitCount == 0 forces the fallback that reads repo.Size; a nil Size must
// not panic and must leave commitSize at 0.
fc := &fakeGitClient{
repo: &git.GitRepository{Size: nil},
commits: map[string]int{"main": 0},
}
branch, size, nbr, err := handleDefaultOrSingleBranch(context.Background(), fc, "proj", "repo", "main", "", "2020-01-01T00:00:00Z")
if err != nil || branch != "main" || size != 0 || nbr != 1 {
t.Fatalf("nil size: got (%q, %d, %d, %v), want (main, 0, 1, nil)", branch, size, nbr, err)
}

// With a Size present, commitSize should reflect it.
var sz uint64 = 512
fc2 := &fakeGitClient{repo: &git.GitRepository{Size: &sz}, commits: map[string]int{"main": 0}}
if _, size, _, err := handleDefaultOrSingleBranch(context.Background(), fc2, "proj", "repo", "main", "", "2020-01-01T00:00:00Z"); err != nil || size != 512 {
t.Fatalf("with size: got (%d, %v), want (512, nil)", size, err)
}
}

// TestGetRepoAnalyse_Fallback drives the getRepoAnalyse error fallback: when
// per-repo branch analysis fails, a repo with a nil default branch is skipped
// (no panic), while a repo with a valid default branch falls back to it.
func TestGetRepoAnalyse_Fallback(t *testing.T) {
id1, id2 := uuid.New(), uuid.New()
fc := &fakeGitClient{
// GetRepository (inside getMostImportantBranch) errors, so branch
// analysis fails for every repo and the fallback path is exercised.
repoErr: context.DeadlineExceeded,
repos: &[]git.GitRepository{
{Id: &id1, Name: strPtr("nildef"), DefaultBranch: nil}, // -> skipped
{Id: &id2, Name: strPtr("hasdef"), DefaultBranch: strPtr("refs/heads/main")}, // -> uses "main"
},
items: &[]git.GitItem{{Path: strPtr("/")}}, // non-empty -> repos not "empty"
branches: branchStats(strPtr("main")), // non-empty
}

spin := spinner.New(spinner.CharSets[35], 100*time.Millisecond)
spin.Writer = io.Discard
params := ParamsProjectAzure{
Context: context.Background(),
Projects: []core.TeamProjectReference{{Name: strPtr("proj1")}},
Organization: "org",
Exclusionlist: utils.NewExclusionList(nil, nil),
Spin: spin,
DefaultB: true,
Period: -1,
}

branches, empty, nbRepos, _, _, _, err := getRepoAnalyse(params, fc)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if empty != 0 || nbRepos != 2 {
t.Errorf("empty=%d nbRepos=%d, want empty=0 nbRepos=2", empty, nbRepos)
}
if len(branches) != 1 {
t.Fatalf("expected 1 analyzed repo (nildef skipped), got %d: %+v", len(branches), branches)
}
if branches[0].RepoSlug != "hasdef" || branches[0].MainBranch != "main" {
t.Errorf("got repo=%q branch=%q, want hasdef/main", branches[0].RepoSlug, branches[0].MainBranch)
}
}

func TestDefaultBranchName(t *testing.T) {
tests := []struct {
name string
repo *git.GitRepository
wantName string
wantOK bool
}{
{"nil repo", nil, "", false},
{"nil default branch", &git.GitRepository{DefaultBranch: nil}, "", false},
{"empty default branch", &git.GitRepository{DefaultBranch: strPtr("")}, "", false},
{"prefixed", &git.GitRepository{DefaultBranch: strPtr("refs/heads/main")}, "main", true},
{"already trimmed", &git.GitRepository{DefaultBranch: strPtr("develop")}, "develop", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotName, gotOK := defaultBranchName(tt.repo)
if gotName != tt.wantName || gotOK != tt.wantOK {
t.Errorf("defaultBranchName() = (%q, %v), want (%q, %v)", gotName, gotOK, tt.wantName, tt.wantOK)
}
})
}
}

func TestIsRepoExcluded(t *testing.T) {
el := utils.NewExclusionList(nil, []string{"PROJ/my-repo", "OTHER/other-repo"})

Expand Down
Loading