From 4cbdacb21cfa2f84350db4d0aea73d5a3579fcd7 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 11 Dec 2021 13:15:04 +0100 Subject: [PATCH] Nits Collected over last month (#595) - add coverage.out - add context queue - fix misspell - sanitize config: WOODPECKER_GITEA_URL - storage backend migration should have no xorm session within migration function --- Makefile | 10 +++---- cmd/agent/agent.go | 2 +- cmd/server/setup.go | 12 ++++++--- server/plugins/secrets/builtin.go | 7 +++-- server/queue/fifo.go | 2 +- server/queue/fifo_test.go | 26 +++++++++---------- .../migration/003_fix_pr_secret_event_name.go | 7 +---- 7 files changed, 35 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index 9ac774032..8594aaccd 100644 --- a/Makefile +++ b/Makefile @@ -51,16 +51,16 @@ lint-frontend: (cd web/; yarn lint --quiet) test-agent: - $(DOCKER_RUN) go test -race -timeout 30s github.com/woodpecker-ci/woodpecker/cmd/agent github.com/woodpecker-ci/woodpecker/agent/... + $(DOCKER_RUN) go test -race -cover -coverprofile coverage.out -timeout 30s github.com/woodpecker-ci/woodpecker/cmd/agent github.com/woodpecker-ci/woodpecker/agent/... test-server: - $(DOCKER_RUN) go test -race -timeout 30s github.com/woodpecker-ci/woodpecker/cmd/server $(shell go list github.com/woodpecker-ci/woodpecker/server/... | grep -v '/store') + $(DOCKER_RUN) go test -race -cover -coverprofile coverage.out -timeout 30s github.com/woodpecker-ci/woodpecker/cmd/server $(shell go list github.com/woodpecker-ci/woodpecker/server/... | grep -v '/store') test-cli: - $(DOCKER_RUN) go test -race -timeout 30s github.com/woodpecker-ci/woodpecker/cmd/cli github.com/woodpecker-ci/woodpecker/cli/... + $(DOCKER_RUN) go test -race -cover -coverprofile coverage.out -timeout 30s github.com/woodpecker-ci/woodpecker/cmd/cli github.com/woodpecker-ci/woodpecker/cli/... test-server-datastore: - $(DOCKER_RUN) go test -timeout 30s github.com/woodpecker-ci/woodpecker/server/store/... + $(DOCKER_RUN) go test -cover -coverprofile coverage.out -timeout 30s github.com/woodpecker-ci/woodpecker/server/store/... test-frontend: frontend-dependencies (cd web/; yarn run lint) @@ -69,7 +69,7 @@ test-frontend: frontend-dependencies (cd web/; yarn run test) test-lib: - $(DOCKER_RUN) go test -race -timeout 30s $(shell go list ./... | grep -v '/cmd\|/agent\|/cli\|/server') + $(DOCKER_RUN) go test -race -cover -coverprofile coverage.out -timeout 30s $(shell go list ./... | grep -v '/cmd\|/agent\|/cli\|/server') test: test-agent test-server test-server-datastore test-cli test-lib test-frontend diff --git a/cmd/agent/agent.go b/cmd/agent/agent.go index badcb6323..415e02229 100644 --- a/cmd/agent/agent.go +++ b/cmd/agent/agent.go @@ -148,7 +148,7 @@ func loop(c *cli.Context) error { return } - // load enginge (e.g. init api client) + // load engine (e.g. init api client) err = engine.Load() if err != nil { log.Error().Err(err).Msg("cannot load backend engine") diff --git a/cmd/server/setup.go b/cmd/server/setup.go index 8de40e3a4..b7cfdeb85 100644 --- a/cmd/server/setup.go +++ b/cmd/server/setup.go @@ -17,7 +17,9 @@ package main import ( "context" "fmt" + "net/url" "os" + "strings" "time" "github.com/prometheus/client_golang/prometheus" @@ -153,11 +155,11 @@ func fallbackSqlite3File(path string) (string, error) { } func setupQueue(c *cli.Context, s store.Store) queue.Queue { - return queue.WithTaskStore(queue.New(), s) + return queue.WithTaskStore(queue.New(c.Context), s) } func setupSecretService(c *cli.Context, s store.Store) model.SecretService { - return secrets.New(s) + return secrets.New(c.Context, s) } func setupRegistryService(c *cli.Context, s store.Store) model.RegistryService { @@ -221,8 +223,12 @@ func setupGogs(c *cli.Context) (remote.Remote, error) { // helper function to setup the Gitea remote from the CLI arguments. func setupGitea(c *cli.Context) (remote.Remote, error) { + server, err := url.Parse(c.String("gitea-server")) + if err != nil { + return nil, err + } opts := gitea.Opts{ - URL: c.String("gitea-server"), + URL: strings.TrimRight(server.String(), "/"), Context: c.String("gitea-context"), Username: c.String("gitea-git-username"), Password: c.String("gitea-git-password"), diff --git a/server/plugins/secrets/builtin.go b/server/plugins/secrets/builtin.go index c3852082b..fab49585d 100644 --- a/server/plugins/secrets/builtin.go +++ b/server/plugins/secrets/builtin.go @@ -1,16 +1,19 @@ package secrets import ( + "context" + "github.com/woodpecker-ci/woodpecker/server/model" ) type builtin struct { + context.Context store model.SecretStore } // New returns a new local secret service. -func New(store model.SecretStore) model.SecretService { - return &builtin{store} +func New(ctx context.Context, store model.SecretStore) model.SecretService { + return &builtin{store: store, Context: ctx} } func (b *builtin) SecretFind(repo *model.Repo, name string) (*model.Secret, error) { diff --git a/server/queue/fifo.go b/server/queue/fifo.go index 0bab454f6..c3b7bfe52 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -42,7 +42,7 @@ type fifo struct { } // New returns a new fifo queue. -func New() Queue { +func New(ctx context.Context) Queue { return &fifo{ workers: map[*worker]struct{}{}, running: map[string]*entry{}, diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index b7d73c8d6..5db7b2ee8 100644 --- a/server/queue/fifo_test.go +++ b/server/queue/fifo_test.go @@ -15,7 +15,7 @@ var noContext = context.Background() func TestFifo(t *testing.T) { want := &Task{ID: "1"} - q := New() + q := New(context.Background()) assert.NoError(t, q.Push(noContext, want)) info := q.Info(noContext) if len(info.Pending) != 1 { @@ -54,7 +54,7 @@ func TestFifo(t *testing.T) { func TestFifoExpire(t *testing.T) { want := &Task{ID: "1"} - q := New().(*fifo) + q := New(context.Background()).(*fifo) q.extension = 0 assert.NoError(t, q.Push(noContext, want)) info := q.Info(noContext) @@ -79,7 +79,7 @@ func TestFifoExpire(t *testing.T) { func TestFifoWait(t *testing.T) { want := &Task{ID: "1"} - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.Push(noContext, want)) got, _ := q.Poll(noContext, func(*Task) bool { return true }) @@ -103,7 +103,7 @@ func TestFifoWait(t *testing.T) { func TestFifoEvict(t *testing.T) { t1 := &Task{ID: "1"} - q := New() + q := New(context.Background()) assert.NoError(t, q.Push(noContext, t1)) info := q.Info(noContext) if len(info.Pending) != 1 { @@ -132,7 +132,7 @@ func TestFifoDependencies(t *testing.T) { DepStatus: make(map[string]string), } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task1})) got, _ := q.Poll(noContext, func(*Task) bool { return true }) @@ -168,7 +168,7 @@ func TestFifoErrors(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task3, task1})) got, _ := q.Poll(noContext, func(*Task) bool { return true }) @@ -217,7 +217,7 @@ func TestFifoErrors2(t *testing.T) { DepStatus: make(map[string]string), } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task3, task1})) for i := 0; i < 2; i++ { @@ -264,7 +264,7 @@ func TestFifoErrorsMultiThread(t *testing.T) { DepStatus: make(map[string]string), } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task3, task1})) obtainedWorkCh := make(chan *Task) @@ -354,7 +354,7 @@ func TestFifoTransitiveErrors(t *testing.T) { DepStatus: make(map[string]string), } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task3, task1})) got, _ := q.Poll(noContext, func(*Task) bool { return true }) @@ -404,7 +404,7 @@ func TestFifoCancel(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task3, task1})) _, _ = q.Poll(noContext, func(*Task) bool { return true }) @@ -424,7 +424,7 @@ func TestFifoPause(t *testing.T) { ID: "1", } - q := New().(*fifo) + q := New(context.Background()).(*fifo) var wg sync.WaitGroup wg.Add(1) go func() { @@ -456,7 +456,7 @@ func TestFifoPauseResume(t *testing.T) { ID: "1", } - q := New().(*fifo) + q := New(context.Background()).(*fifo) q.Pause() assert.NoError(t, q.Push(noContext, task1)) q.Resume() @@ -482,7 +482,7 @@ func TestWaitingVsPending(t *testing.T) { RunOn: []string{"success", "failure"}, } - q := New().(*fifo) + q := New(context.Background()).(*fifo) assert.NoError(t, q.PushAtOnce(noContext, []*Task{task2, task3, task1})) got, _ := q.Poll(noContext, func(*Task) bool { return true }) diff --git a/server/store/datastore/migration/003_fix_pr_secret_event_name.go b/server/store/datastore/migration/003_fix_pr_secret_event_name.go index b7366613c..a7f104a39 100644 --- a/server/store/datastore/migration/003_fix_pr_secret_event_name.go +++ b/server/store/datastore/migration/003_fix_pr_secret_event_name.go @@ -45,11 +45,6 @@ var fixPRSecretEventName = task{ } } } - - if err := sess.Begin(); err != nil { - return err - } - - return sess.Commit() + return nil }, }