Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly count the number of repos after a github scan resume #625

Merged
merged 2 commits into from
Jun 17, 2022
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
6 changes: 3 additions & 3 deletions pkg/sources/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ func (s *Source) scan(ctx context.Context, installationClient *github.Client, ch
defer s.jobSem.Release(1)
defer wg.Done()

s.setProgressCompleteWithRepo(i+progressIndexOffset, repoURL)
s.setProgressCompleteWithRepo(i, progressIndexOffset, repoURL)
// Ensure the repo is removed from the resume info after being scanned.
defer func(s *Source) {
s.resumeInfoMutex.Lock()
Expand Down Expand Up @@ -794,7 +794,7 @@ func (s *Source) normalizeRepos(ctx context.Context, apiClient *github.Client) {
}

// setProgressCompleteWithRepo calls the s.SetProgressComplete after safely setting up the encoded resume info string.
func (s *Source) setProgressCompleteWithRepo(index int, repoURL string) {
func (s *Source) setProgressCompleteWithRepo(index int, offset int, repoURL string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (optional): Same suggestion as the gitlab PR, it would be nice to have a test to prevent regressions.

s.resumeInfoMutex.Lock()
defer s.resumeInfoMutex.Unlock()

Expand All @@ -805,5 +805,5 @@ func (s *Source) setProgressCompleteWithRepo(index int, repoURL string) {
// Make the resume info string from the slice.
encodedResumeInfo := sources.EncodeResumeInfo(s.resumeInfoSlice)

s.SetProgressComplete(index, len(s.repos), fmt.Sprintf("Repo: %s", repoURL), encodedResumeInfo)
s.SetProgressComplete(index+offset, len(s.repos)+offset, fmt.Sprintf("Repo: %s", repoURL), encodedResumeInfo)
}
63 changes: 61 additions & 2 deletions pkg/sources/github/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ func TestEnumerateWithApp(t *testing.T) {
}

// This only tests the resume info slice portion of setProgressCompleteWithRepo.
func Test_setProgressCompleteWithRepo(t *testing.T) {
func Test_setProgressCompleteWithRepo_resumeInfo(t *testing.T) {
tests := []struct {
startingResumeInfoSlice []string
repoURL string
Expand All @@ -367,9 +367,68 @@ func Test_setProgressCompleteWithRepo(t *testing.T) {

for _, tt := range tests {
s.resumeInfoSlice = tt.startingResumeInfoSlice
s.setProgressCompleteWithRepo(0, tt.repoURL)
s.setProgressCompleteWithRepo(0, 0, tt.repoURL)
if !reflect.DeepEqual(s.resumeInfoSlice, tt.wantResumeInfoSlice) {
t.Errorf("s.setProgressCompleteWithRepo() got: %v, want: %v", s.resumeInfoSlice, tt.wantResumeInfoSlice)
}
}
}

func Test_setProgressCompleteWithRepo_Progress(t *testing.T) {
repos := []string{"a", "b", "c", "d", "e"}
tests := map[string]struct {
repos []string
index int
offset int
wantPercentComplete int64
wantSectionsCompleted int32
wantSectionsRemaining int32
}{
"starting from the beginning, no offset": {
repos: repos,
index: 0,
offset: 0,
wantPercentComplete: 0,
wantSectionsCompleted: 0,
wantSectionsRemaining: 5,
},
"resume from the third, offset 2": {
repos: repos[2:],
index: 0,
offset: 2,
wantPercentComplete: 40,
wantSectionsCompleted: 2,
wantSectionsRemaining: 5,
},
"resume from the third, on last repo, offset 2": {
repos: repos[2:],
index: 2,
offset: 2,
wantPercentComplete: 80,
wantSectionsCompleted: 4,
wantSectionsRemaining: 5,
},
}

logger := logrus.New()
logger.Out = io.Discard

for _, tt := range tests {
s := &Source{
repos: tt.repos,
log: logger.WithField("no", "output"),
}

s.setProgressCompleteWithRepo(tt.index, tt.offset, "")
gotProgress := s.GetProgress()
if gotProgress.PercentComplete != tt.wantPercentComplete {
t.Errorf("s.setProgressCompleteWithRepo() PercentComplete got: %v want: %v", gotProgress.PercentComplete, tt.wantPercentComplete)
}
if gotProgress.SectionsCompleted != tt.wantSectionsCompleted {
t.Errorf("s.setProgressCompleteWithRepo() PercentComplete got: %v want: %v", gotProgress.SectionsCompleted, tt.wantSectionsCompleted)
}
if gotProgress.SectionsRemaining != tt.wantSectionsRemaining {
t.Errorf("s.setProgressCompleteWithRepo() PercentComplete got: %v want: %v", gotProgress.SectionsRemaining, tt.wantSectionsRemaining)
}
}
}