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

@@ -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
 

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")

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"),

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) {

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{},

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 })

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
 	},
 }