Skip to content
This repository has been archived by the owner on Aug 6, 2021. It is now read-only.

Implement cooldowns #10

Merged
merged 2 commits into from
Oct 12, 2020
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
5 changes: 0 additions & 5 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ linters:
- dupl
- errcheck
- exhaustive
- funlen
- goconst
- gocritic
- gocyclo
Expand Down Expand Up @@ -54,7 +53,3 @@ issues:
exclude:
- G101
- G306
exclude-rules:
- path: _test\.go
linters:
- funlen
4 changes: 4 additions & 0 deletions repo/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,3 +282,7 @@ func (t *GitRepo) push(ctx context.Context) error {
logrus.Debug("pushed to remote")
return nil
}

func (t *GitRepo) ExistingUpdates(context.Context, string) (updater.ExistingUpdates, error) {
return nil, nil
}
4 changes: 2 additions & 2 deletions repo/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ func (g *GitHubRepo) createPR(ctx context.Context, updates updater.UpdateGroup)
return nil
}

func (g *GitHubRepo) ExistingUpdates(ctx context.Context, baseBranch string) ([]updater.ExistingUpdate, error) {
func (g *GitHubRepo) ExistingUpdates(ctx context.Context, baseBranch string) (updater.ExistingUpdates, error) {
return g.updateFromPR(ctx, baseBranch)
}

func (g *GitHubRepo) updateFromPR(ctx context.Context, baseBranch string) ([]updater.ExistingUpdate, error) {
func (g *GitHubRepo) updateFromPR(ctx context.Context, baseBranch string) (updater.ExistingUpdates, error) {
// List pull requests targeting this branch:
prs, _, err := g.github.PullRequests.List(ctx, g.owner, g.repoName, &github.PullRequestListOptions{
State: "all",
Expand Down
23 changes: 23 additions & 0 deletions updater/mockrepo_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions updater/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,17 @@ type ExistingUpdate struct {
LastUpdate time.Time
Group UpdateGroup
}

type ExistingUpdates []ExistingUpdate

func (e ExistingUpdates) LatestGroupUpdate(group string) (latest time.Time) {
for _, u := range e {
if u.Group.Name != group {
continue
}
if u.LastUpdate.After(latest) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

TBD: this is literally "last update".

Maybe CreatedAt is better?

latest = u.LastUpdate
}
}
return
}
31 changes: 27 additions & 4 deletions updater/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"
"os/exec"
"time"

"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -33,6 +34,8 @@ type Repo interface {
Push(context.Context, UpdateGroup) error
// Fetch loads a remote ref without updating the working copy.
Fetch(ctx context.Context, branch string) error
// ExistingUpdates returns the state of recent updates
ExistingUpdates(ctx context.Context, baseBranch string) (ExistingUpdates, error)
}

type Updater interface {
Expand Down Expand Up @@ -113,6 +116,13 @@ func (u *RepoUpdater) updateBranch(ctx context.Context, log logrus.FieldLogger,
return fmt.Errorf("getting dependencies: %w", err)
}

// Load existing updates
existing, err := u.repo.ExistingUpdates(ctx, branch)
if err != nil {
return fmt.Errorf("fetching existing updates: %w", err)
}

// Cluster dependencies into groups:
groups, ungrouped := u.groups.GroupDependencies(deps)
log.WithFields(logrus.Fields{
"deps": len(deps),
Expand All @@ -122,14 +132,27 @@ func (u *RepoUpdater) updateBranch(ctx context.Context, log logrus.FieldLogger,

updates := 0
for groupName, groupDeps := range groups {
groupLog := log.WithField("group", groupName)

if cd := u.groups.ByName(groupName).CoolDownDuration(); cd > 0 {
if latest := existing.LatestGroupUpdate(groupName); !latest.IsZero() {
ageOfLatest := time.Since(latest)
if ageOfLatest < cd {
groupLog.WithFields(logrus.Fields{
"cooldown": cd,
"age": ageOfLatest,
}).Info("skipping group in cooldown")
continue
}
}
}

groupLog.WithField("deps", len(groupDeps)).Debug("checking update group")
groupUpdates, err := u.groupedUpdate(ctx, log, branch, groupName, groupDeps)
if err != nil {
return err
}
log.WithFields(logrus.Fields{
"group": groupName,
"updates": updates,
}).Debug("checked update group")
groupLog.WithField("updates", groupUpdates).Debug("checked update group")
updates += groupUpdates
}

Expand Down
35 changes: 35 additions & 0 deletions updater/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"path/filepath"
"testing"
"time"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand All @@ -25,6 +26,8 @@ func TestRepoUpdater_Update(t *testing.T) {

err := ru.Update(ctx, baseBranch, branch, updater.NewUpdateGroup("", mockUpdate))
require.NoError(t, err)
r.AssertExpectations(t)
u.AssertExpectations(t)
}

func setupMockUpdate(ctx context.Context, r *mockRepo, u *mockUpdater, up updater.Update) string {
Expand All @@ -44,6 +47,7 @@ func TestRepoUpdater_UpdateAll_NoChanges(t *testing.T) {
r.On("SetBranch", baseBranch).Return(nil)
dep := updater.Dependency{Path: mockUpdate.Path, Version: mockUpdate.Previous}
u.On("Dependencies", ctx).Return([]updater.Dependency{dep}, nil)
r.On("ExistingUpdates", ctx, baseBranch).Return(nil, nil)
u.On("Check", ctx, dep, mock.Anything).Return(nil, nil)

err := ru.UpdateAll(ctx, baseBranch)
Expand All @@ -61,6 +65,7 @@ func TestRepoUpdater_UpdateAll_Update(t *testing.T) {
r.On("SetBranch", baseBranch).Return(nil)
dep := updater.Dependency{Path: mockUpdate.Path, Version: mockUpdate.Previous}
u.On("Dependencies", ctx).Return([]updater.Dependency{dep}, nil)
r.On("ExistingUpdates", ctx, baseBranch).Return(nil, nil)
availableUpdate := mockUpdate // avoid pointer to shared reference
u.On("Check", ctx, dep, mock.Anything).Return(&availableUpdate, nil)
setupMockUpdate(ctx, r, u, mockUpdate) // delegates to .Update()
Expand All @@ -81,6 +86,7 @@ func TestRepoUpdater_UpdateAll_Multiple(t *testing.T) {
dep := updater.Dependency{Path: mockUpdate.Path, Version: mockUpdate.Previous}
otherDep := updater.Dependency{Path: "github.com/foo/baz", Version: mockUpdate.Previous}
u.On("Dependencies", ctx).Return([]updater.Dependency{dep, otherDep}, nil)
r.On("ExistingUpdates", ctx, baseBranch).Return(nil, nil)
availableUpdate := mockUpdate // avoid pointer to shared reference
u.On("Check", ctx, dep, mock.Anything).Return(&availableUpdate, nil)
otherUpdate := updater.Update{Path: "github.com/foo/baz", Next: "v3.0.0"}
Expand Down Expand Up @@ -108,6 +114,7 @@ func TestRepoUpdater_UpdateAll_MultipleGrouped(t *testing.T) {
dep := updater.Dependency{Path: mockUpdate.Path, Version: mockUpdate.Previous}
otherDep := updater.Dependency{Path: "github.com/foo/baz", Version: mockUpdate.Previous}
u.On("Dependencies", ctx).Return([]updater.Dependency{dep, otherDep}, nil)
r.On("ExistingUpdates", ctx, baseBranch).Return(nil, nil)
availableUpdate := mockUpdate // avoid pointer to shared reference
u.On("Check", ctx, dep, mock.Anything).Return(&availableUpdate, nil)
otherUpdate := updater.Update{Path: "github.com/foo/baz", Next: "v3.0.0"}
Expand Down Expand Up @@ -151,6 +158,7 @@ func TestRepoUpdater_UpdateAll_Scripts(t *testing.T) {
r.On("SetBranch", baseBranch).Return(nil)
dep := updater.Dependency{Path: mockUpdate.Path, Version: mockUpdate.Previous}
u.On("Dependencies", ctx).Return([]updater.Dependency{dep}, nil)
r.On("ExistingUpdates", ctx, baseBranch).Return(nil, nil)
availableUpdate := mockUpdate // avoid pointer to shared reference
u.On("Check", ctx, dep, mock.Anything).Return(&availableUpdate, nil)
r.On("NewBranch", baseBranch, "action-update-go/main/foo").Times(1).Return(nil)
Expand All @@ -166,3 +174,30 @@ func TestRepoUpdater_UpdateAll_Scripts(t *testing.T) {
require.NoError(t, err)
}
}

func TestRepoUpdater_UpdateAll_CoolDown(t *testing.T) {
// Group with 1 day cooldown
group := &updater.Group{Name: groupName, Pattern: "github.com/foo", CoolDown: "1D"}
err := group.Validate()
require.NoError(t, err)

r := &mockRepo{}
u := &mockUpdater{}
ru := updater.NewRepoUpdater(r, u, updater.WithGroups(group))
ctx := context.Background()
r.On("SetBranch", baseBranch).Return(nil)
dep := updater.Dependency{Path: mockUpdate.Path, Version: mockUpdate.Previous}
otherDep := updater.Dependency{Path: "github.com/foo/baz", Version: mockUpdate.Previous}
u.On("Dependencies", ctx).Return([]updater.Dependency{dep, otherDep}, nil)

// Existing update within 30m means update is not checked:
existingUpdates := updater.ExistingUpdates{
{Group: updater.UpdateGroup{Name: groupName}, Merged: true, LastUpdate: time.Now().Add(-1 * time.Hour)},
}
r.On("ExistingUpdates", ctx, baseBranch).Return(existingUpdates, nil)

err = ru.UpdateAll(ctx, baseBranch)
require.NoError(t, err)
r.AssertExpectations(t)
u.AssertExpectations(t)
}