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.

Sign up for GitHub

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

Run CI on all supported versions of Go #356

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Run CI on all supported versions of Go #356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 14 additions & 5 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,24 @@ on:
branches:
- master
pull_request:

name: CI
jobs:
test:
name: Test
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
strategy:
fail-fast: false
matrix:
go-version: ["1.17", "1.18", "1.19", "1.20", "1.21", "1.22", "stable"]
steps:
- name: Checkout code
- name: Checkout Repository
uses: actions/checkout@v4
- name: Init Hermit
run: ./bin/hermit env -r >> $GITHUB_ENV

- name: Setup Golang Environment
uses: actions/setup-go@v5
Copy link

Choose a reason for hiding this comment

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

Minor nit, It's recommended to pin this to a git hash for supply chain security.

Renovate can also keep this up to date.

Suggested change
uses: actions/setup-go@v5
uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah but this repo is not really maintained...I'm not sure who would merge all the PRs...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean I’d be down to help maintain it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alecthomas ?

Copy link
Owner

Choose a reason for hiding this comment

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

I appreciate the effort but I personally don't think it's useful to test on historical versions of Go at all, for multiple reasons:

  1. Only the most recent 2 versions are supported by the Go team, and I generally adhere to this policy too.
  2. The Go backwards compatibility promise is incredibly reliable.

The other changes seem reasonable.

Copy link

Choose a reason for hiding this comment

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

My usual recommendation here is to update the go directive in the go.mod file to support the latest 2 releases. This way the matrix test matches. This is the policy we use for most of the Prometheus ecosystem.

with:
go-version: ${{ matrix.go-version }}

- name: Test
run: go test ./...
run: go test ./... -race -shuffle=on -v
34 changes: 28 additions & 6 deletions app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@ package kingpin

import (
"errors"
"io/ioutil"

"github.com/stretchr/testify/assert"

"io"
"sort"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

func newTestApp() *Application {
return New("test", "").Terminate(nil)
}

func TestCommander(t *testing.T) {
t.Parallel()
c := newTestApp()
ping := c.Command("ping", "Ping an IP address.")
pingTTL := ping.Flag("ttl", "TTL for ICMP packets").Short('t').Default("5s").Duration()
Expand All @@ -33,6 +33,7 @@ func TestCommander(t *testing.T) {
}

func TestRequiredFlags(t *testing.T) {
t.Parallel()
c := newTestApp()
c.Flag("a", "a").String()
c.Flag("b", "b").Required().String()
Expand All @@ -44,6 +45,7 @@ func TestRequiredFlags(t *testing.T) {
}

func TestRepeatableFlags(t *testing.T) {
t.Parallel()
c := newTestApp()
c.Flag("a", "a").String()
c.Flag("b", "b").Strings()
Expand All @@ -54,13 +56,15 @@ func TestRepeatableFlags(t *testing.T) {
}

func TestInvalidDefaultFlagValueErrors(t *testing.T) {
t.Parallel()
c := newTestApp()
c.Flag("foo", "foo").Default("a").Int()
_, err := c.Parse([]string{})
assert.Error(t, err)
}

func TestInvalidDefaultArgValueErrors(t *testing.T) {
t.Parallel()
c := newTestApp()
cmd := c.Command("cmd", "cmd")
cmd.Arg("arg", "arg").Default("one").Int()
Expand All @@ -69,6 +73,7 @@ func TestInvalidDefaultArgValueErrors(t *testing.T) {
}

func TestArgsRequiredAfterNonRequiredErrors(t *testing.T) {
t.Parallel()
c := newTestApp()
cmd := c.Command("cmd", "")
cmd.Arg("a", "a").String()
Expand All @@ -78,7 +83,8 @@ func TestArgsRequiredAfterNonRequiredErrors(t *testing.T) {
}

func TestArgsMultipleRequiredThenNonRequired(t *testing.T) {
c := newTestApp().Writer(ioutil.Discard)
t.Parallel()
c := newTestApp().Writer(io.Discard)
cmd := c.Command("cmd", "")
cmd.Arg("a", "a").Required().String()
cmd.Arg("b", "b").Required().String()
Expand All @@ -91,6 +97,7 @@ func TestArgsMultipleRequiredThenNonRequired(t *testing.T) {
}

func TestDispatchCallbackIsCalled(t *testing.T) {
t.Parallel()
dispatched := false
c := newTestApp()
c.Command("cmd", "").Action(func(*ParseContext) error {
Expand All @@ -104,6 +111,7 @@ func TestDispatchCallbackIsCalled(t *testing.T) {
}

func TestTopLevelArgWorks(t *testing.T) {
t.Parallel()
c := newTestApp()
s := c.Arg("arg", "help").String()
_, err := c.Parse([]string{"foo"})
Expand All @@ -112,6 +120,7 @@ func TestTopLevelArgWorks(t *testing.T) {
}

func TestTopLevelArgCantBeUsedWithCommands(t *testing.T) {
t.Parallel()
c := newTestApp()
c.Arg("arg", "help").String()
c.Command("cmd", "help")
Expand All @@ -120,13 +129,15 @@ func TestTopLevelArgCantBeUsedWithCommands(t *testing.T) {
}

func TestTooManyArgs(t *testing.T) {
t.Parallel()
a := newTestApp()
a.Arg("a", "").String()
_, err := a.Parse([]string{"a", "b"})
assert.Error(t, err)
}

func TestTooManyArgsAfterCommand(t *testing.T) {
t.Parallel()
a := newTestApp()
a.Command("a", "")
assert.NoError(t, a.init())
Expand All @@ -135,13 +146,15 @@ func TestTooManyArgsAfterCommand(t *testing.T) {
}

func TestArgsLooksLikeFlagsWithConsumeRemainder(t *testing.T) {
t.Parallel()
a := newTestApp()
a.Arg("opts", "").Required().Strings()
_, err := a.Parse([]string{"hello", "-world"})
assert.Error(t, err)
}

func TestCommandParseDoesNotResetFlagsToDefault(t *testing.T) {
t.Parallel()
app := newTestApp()
flag := app.Flag("flag", "").Default("default").String()
app.Command("cmd", "")
Expand All @@ -152,6 +165,7 @@ func TestCommandParseDoesNotResetFlagsToDefault(t *testing.T) {
}

func TestCommandParseDoesNotFailRequired(t *testing.T) {
t.Parallel()
app := newTestApp()
flag := app.Flag("flag", "").Required().String()
app.Command("cmd", "")
Expand All @@ -162,6 +176,7 @@ func TestCommandParseDoesNotFailRequired(t *testing.T) {
}

func TestSelectedCommand(t *testing.T) {
t.Parallel()
app := newTestApp()
c0 := app.Command("c0", "")
c0.Command("c1", "")
Expand All @@ -171,6 +186,7 @@ func TestSelectedCommand(t *testing.T) {
}

func TestSubCommandRequired(t *testing.T) {
t.Parallel()
app := newTestApp()
c0 := app.Command("c0", "")
c0.Command("c1", "")
Expand All @@ -179,6 +195,7 @@ func TestSubCommandRequired(t *testing.T) {
}

func TestInterspersedFalse(t *testing.T) {
t.Parallel()
app := newTestApp().Interspersed(false)
a1 := app.Arg("a1", "").String()
a2 := app.Arg("a2", "").String()
Expand All @@ -192,6 +209,7 @@ func TestInterspersedFalse(t *testing.T) {
}

func TestInterspersedTrue(t *testing.T) {
t.Parallel()
// test once with the default value and once with explicit true
for i := 0; i < 2; i++ {
app := newTestApp()
Expand All @@ -214,6 +232,7 @@ func TestInterspersedTrue(t *testing.T) {
}

func TestDefaultEnvars(t *testing.T) {
t.Parallel()
a := New("some-app", "").Terminate(nil).DefaultEnvars()
f0 := a.Flag("some-flag", "")
f0.Bool()
Expand All @@ -229,6 +248,7 @@ func TestDefaultEnvars(t *testing.T) {
}

func TestBashCompletionOptionsWithEmptyApp(t *testing.T) {
t.Parallel()
a := newTestApp()
context, err := a.ParseContext([]string{"--completion-bash"})
if err != nil {
Expand All @@ -239,6 +259,7 @@ func TestBashCompletionOptionsWithEmptyApp(t *testing.T) {
}

func TestBashCompletionOptions(t *testing.T) {
t.Parallel()
a := newTestApp()
a.Command("one", "")
a.Flag("flag-0", "").String()
Expand Down Expand Up @@ -411,10 +432,10 @@ func TestBashCompletionOptions(t *testing.T) {

assert.Equal(t, c.ExpectedOptions, args, "Expected != Actual: [%v] != [%v]. \nInput was: [%v]", c.ExpectedOptions, args, c.Args)
}

}

func TestCmdValidation(t *testing.T) {
t.Parallel()
c := newTestApp()
cmd := c.Command("cmd", "")

Expand All @@ -436,6 +457,7 @@ func TestCmdValidation(t *testing.T) {
}

func TestVersion(t *testing.T) {
t.Parallel()
c := newTestApp()
c.Flag("config", "path to config file").Default("config.yaml").ExistingFile()
c.Version("1.0.0")
Expand Down
15 changes: 10 additions & 5 deletions args_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package kingpin

import (
"io/ioutil"
"os"
"io"
"testing"

"github.com/stretchr/testify/assert"
)

func TestArgRemainder(t *testing.T) {
t.Parallel()
app := New("test", "")
v := app.Arg("test", "").Strings()
args := []string{"hello", "world"}
Expand All @@ -18,16 +18,18 @@ func TestArgRemainder(t *testing.T) {
}

func TestArgRemainderErrorsWhenNotLast(t *testing.T) {
t.Parallel()
a := newArgGroup()
a.Arg("test", "").Strings()
a.Arg("test2", "").String()
assert.Error(t, a.init())
}

func TestArgMultipleRequired(t *testing.T) {
t.Parallel()
terminated := false
app := New("test", "")
app.Version("0.0.0").Writer(ioutil.Discard)
app.Version("0.0.0").Writer(io.Discard)
app.Arg("a", "").Required().String()
app.Arg("b", "").Required().String()
app.Terminate(func(int) { terminated = true })
Expand All @@ -43,13 +45,15 @@ func TestArgMultipleRequired(t *testing.T) {
}

func TestInvalidArgsDefaultCanBeOverridden(t *testing.T) {
t.Parallel()
app := New("test", "")
app.Arg("a", "").Default("invalid").Bool()
_, err := app.Parse([]string{})
assert.Error(t, err)
}

func TestArgMultipleValuesDefault(t *testing.T) {
t.Parallel()
app := New("test", "")
a := app.Arg("a", "").Default("default1", "default2").Strings()
_, err := app.Parse([]string{})
Expand All @@ -58,14 +62,15 @@ func TestArgMultipleValuesDefault(t *testing.T) {
}

func TestRequiredArgWithEnvarMissingErrors(t *testing.T) {
t.Parallel()
app := newTestApp()
app.Arg("t", "").Envar("TEST_ARG_ENVAR").Required().Int()
_, err := app.Parse([]string{})
assert.Error(t, err)
}

func TestArgRequiredWithEnvar(t *testing.T) {
os.Setenv("TEST_ARG_ENVAR", "123")
t.Setenv("TEST_ARG_ENVAR", "123")
app := newTestApp()
flag := app.Arg("t", "").Envar("TEST_ARG_ENVAR").Required().Int()
_, err := app.Parse([]string{})
Expand All @@ -74,7 +79,7 @@ func TestArgRequiredWithEnvar(t *testing.T) {
}

func TestSubcommandArgRequiredWithEnvar(t *testing.T) {
os.Setenv("TEST_ARG_ENVAR", "123")
t.Setenv("TEST_ARG_ENVAR", "123")
app := newTestApp()
cmd := app.Command("command", "")
flag := cmd.Arg("t", "").Envar("TEST_ARG_ENVAR").Required().Int()
Expand Down
Loading