Skip to content

Commit

Permalink
use Go standard errors (#619)
Browse files Browse the repository at this point in the history
Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 authored Oct 14, 2024
1 parent 0f4277d commit 7427925
Show file tree
Hide file tree
Showing 15 changed files with 206 additions and 190 deletions.
25 changes: 21 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
issues:
max-same-issues: 0

linters:
enable:
- errorlint
- gomodguard

linters-settings:
gomodguard:
blocked:
modules:
- github.com/pkg/errors:
recommendations:
- errors
- fmt

output:
sort-results: true

run:
timeout: 5m
skip-files:
# Skip autogenerated files.
- ^.*\.(pb|y)\.go$

output:
sort-results: true
timeout: 5m
4 changes: 2 additions & 2 deletions funcbench/bench.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package main
import (
"bytes"
"encoding/base64"
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -23,7 +24,6 @@ import (

"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
"github.com/pkg/errors"
"golang.org/x/perf/benchstat"
)

Expand Down Expand Up @@ -101,7 +101,7 @@ func (b *Benchmarker) exec(pkgRoot string, commit plumbing.Hash) (string, error)
}
out, err := b.c.exec(benchCmd...)
if err != nil {
return "", errors.Wrap(err, "benchmark ended with an error.")
return "", fmt.Errorf("benchmark ended with an error.: %w", err)
}

fn := filepath.Join(b.resultCacheDir, fileName)
Expand Down
12 changes: 6 additions & 6 deletions funcbench/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package main
import (
"bytes"
"context"
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -25,7 +26,6 @@ import (
"github.com/go-git/go-git/v5/config"
"github.com/go-git/go-git/v5/plumbing"
"github.com/google/go-github/v29/github"
"github.com/pkg/errors"
"golang.org/x/oauth2"
"golang.org/x/perf/benchstat"
)
Expand Down Expand Up @@ -122,11 +122,11 @@ func newGitHubEnv(ctx context.Context, e environment, gc *gitHubClient, workspac
}
}
if err != nil {
return nil, errors.Wrap(err, "clone git repository")
return nil, fmt.Errorf("clone git repository: %w", err)
}

if err := os.Chdir(filepath.Join(workspace, gc.repo)); err != nil {
return nil, errors.Wrapf(err, "changing to %s/%s dir", workspace, gc.repo)
return nil, fmt.Errorf("changing to %s/%s dir: %w", workspace, gc.repo, err)
}

g := &GitHub{
Expand All @@ -150,14 +150,14 @@ func newGitHubEnv(ctx context.Context, e environment, gc *gitHubClient, workspac
config.RefSpec(fmt.Sprintf("+refs/pull/%d/head:refs/heads/pullrequest", gc.prNumber)),
},
Progress: os.Stdout,
}); err != nil && err != git.NoErrAlreadyUpToDate {
return nil, errors.Wrap(err, "fetch to pull request branch")
}); err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) {
return nil, fmt.Errorf("fetch to pull request branch: %w", err)
}

if err = wt.Checkout(&git.CheckoutOptions{
Branch: plumbing.NewBranchReferenceName("pullrequest"),
}); err != nil {
return nil, errors.Wrap(err, "switch to pull request branch")
return nil, fmt.Errorf("switch to pull request branch: %w", err)
}

e.logger.Println("[GitHub Mode]", gc.owner, ":", gc.repo, "\nBenchmarking PR -", gc.prNumber, "versus:", e.compareTarget, "\nBenchmark func regex:", e.benchFunc)
Expand Down
36 changes: 18 additions & 18 deletions funcbench/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package main
import (
"bytes"
"context"
"errors"
"fmt"
"io"
"log"
Expand All @@ -30,7 +31,6 @@ import (
"github.com/go-git/go-git/v5"
"github.com/go-git/go-git/v5/plumbing"
"github.com/oklog/run"
"github.com/pkg/errors"
"golang.org/x/perf/benchstat"
"gopkg.in/alecthomas/kingpin.v2"
)
Expand Down Expand Up @@ -155,21 +155,21 @@ func main() {
// Local Mode.
env, err = newLocalEnv(e)
if err != nil {
return errors.Wrap(err, "environment create")
return fmt.Errorf("environment create: %w", err)
}
} else {
// Github Mode.
ghClient, err := newGitHubClient(ctx, cfg.owner, cfg.repo, cfg.ghPR, cfg.nocomment)
if err != nil {
return errors.Wrapf(err, "github client")
return fmt.Errorf("github client: %w", err)
}

env, err = newGitHubEnv(ctx, e, ghClient, cfg.workspaceDir)
if err != nil {
if err := ghClient.postComment(fmt.Sprintf("%v. Could not setup environment, please check logs.", err)); err != nil {
return errors.Wrap(err, "could not post error")
return fmt.Errorf("could not post error: %w", err)
}
return errors.Wrap(err, "environment create")
return fmt.Errorf("environment create: %w", err)
}
}

Expand All @@ -192,7 +192,7 @@ func main() {
)

if pErr != nil {
return errors.Wrap(pErr, "could not log error")
return fmt.Errorf("could not log error: %w", pErr)
}
return err
}
Expand All @@ -219,7 +219,7 @@ func main() {
}

if err := g.Run(); err != nil {
logger.FatalError(errors.Wrap(err, "running command failed"))
logger.FatalError(fmt.Errorf("running command failed: %w", err))
}
logger.Println("exiting")
}
Expand All @@ -237,24 +237,24 @@ func startBenchmark(env Environment, bench *Benchmarker) ([]*benchstat.Table, er

ref, err := env.Repo().Head()
if err != nil {
return nil, errors.Wrap(err, "get head")
return nil, fmt.Errorf("get head: %w", err)
}

// TODO move it into env? since GitHub env doesn't need this check.
if _, err := bench.c.exec("sh", "-c", "git update-index -q --ignore-submodules --refresh && git diff-files --quiet --ignore-submodules --"); err != nil {
return nil, errors.Wrap(err, "not clean worktree")
return nil, fmt.Errorf("not clean worktree: %w", err)
}

if env.CompareTarget() == "." {
bench.logger.Println("Assuming sub-benchmarks comparison.")
subResult, err := bench.exec(wt.Filesystem.Root(), ref.Hash())
if err != nil {
return nil, errors.Wrap(err, "execute sub-benchmark")
return nil, fmt.Errorf("execute sub-benchmark: %w", err)
}

cmps, err := bench.compareSubBenchmarks(subResult)
if err != nil {
return nil, errors.Wrap(err, "comparing sub benchmarks")
return nil, fmt.Errorf("comparing sub benchmarks: %w", err)
}
return cmps, nil
}
Expand All @@ -276,35 +276,35 @@ func startBenchmark(env Environment, bench *Benchmarker) ([]*benchstat.Table, er
// Execute benchmark A.
newResult, err := bench.exec(wt.Filesystem.Root(), ref.Hash())
if err != nil {
return nil, errors.Wrapf(err, "execute benchmark for A: %v", ref.Name().String())
return nil, fmt.Errorf("execute benchmark for A: %v: %w", ref.Name().String(), err)
}

// TODO move the following part before 'Execute benchmark B.' into a function Benchmarker.switchToWorkTree.
// Best effort cleanup and checkout new worktree.
if err := os.RemoveAll(cmpWorkTreeDir); err != nil {
return nil, errors.Wrapf(err, "delete worktree at %s", cmpWorkTreeDir)
return nil, fmt.Errorf("delete worktree at %s: %w", cmpWorkTreeDir, err)
}

// TODO (geekodour): switch to worktree remove once we decide not to support git<2.17
if _, err := bench.c.exec("git", "worktree", "prune"); err != nil {
return nil, errors.Wrap(err, "worktree prune")
return nil, fmt.Errorf("worktree prune: %w", err)
}

bench.logger.Println("Checking out (in new workdir):", cmpWorkTreeDir, "commmit", targetCommit.String())
if _, err := bench.c.exec("git", "worktree", "add", "-f", cmpWorkTreeDir, targetCommit.String()); err != nil {
return nil, errors.Wrapf(err, "checkout %s in worktree %s", targetCommit.String(), cmpWorkTreeDir)
return nil, fmt.Errorf("checkout %s in worktree %s: %w", targetCommit.String(), cmpWorkTreeDir, err)
}

// Execute benchmark B.
oldResult, err := bench.exec(cmpWorkTreeDir, targetCommit)
if err != nil {
return nil, errors.Wrapf(err, "execute benchmark for B: %v", env.CompareTarget())
return nil, fmt.Errorf("execute benchmark for B: %v: %w", env.CompareTarget(), err)
}

// Compare B vs A.
tables, err := compareBenchmarks(oldResult, newResult)
if err != nil {
return nil, errors.Wrap(err, "comparing benchmarks")
return nil, fmt.Errorf("comparing benchmarks: %w", err)
}

// Save hashes for info about benchmark.
Expand Down Expand Up @@ -354,7 +354,7 @@ func (c *commander) exec(command ...string) (string, error) {
}
if err := cmd.Run(); err != nil {
out := b.String()
return "", errors.Errorf("error: %v; Command out: %s", err, out)
return "", fmt.Errorf("error: %w; Command out: %s", err, out)
}

return b.String(), nil
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ require (
github.com/go-git/go-git/v5 v5.12.0
github.com/google/go-github/v29 v29.0.3
github.com/oklog/run v1.1.0
github.com/pkg/errors v0.9.1
github.com/prometheus/alertmanager v0.27.0
github.com/prometheus/client_golang v1.20.4
github.com/prometheus/common v0.60.0
Expand Down Expand Up @@ -108,6 +107,7 @@ require (
github.com/onsi/gomega v1.33.1 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pjbgf/sha1cd v0.3.0 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common/sigv4 v0.1.0 // indirect
github.com/prometheus/exporter-toolkit v0.11.0 // indirect
Expand Down
3 changes: 1 addition & 2 deletions infra/infra.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"os"
"path/filepath"

"github.com/pkg/errors"
"gopkg.in/alecthomas/kingpin.v2"

"github.com/prometheus/test-infra/pkg/provider"
Expand Down Expand Up @@ -154,7 +153,7 @@ func main() {
Action(e.ResourceDelete)

if _, err := app.Parse(os.Args[1:]); err != nil {
fmt.Fprintln(os.Stderr, errors.Wrapf(err, "Error parsing commandline arguments"))
fmt.Fprintln(os.Stderr, fmt.Errorf("Error parsing commandline arguments: %w", err))
app.Usage(os.Args[1:])
os.Exit(2)
}
Expand Down
Loading

0 comments on commit 7427925

Please sign in to comment.