Skip to content

Commit 5700952

Browse files
committed
Do not print unsupported mixin command errors
Not all of the mixin commands are required, for example schema and lint are optional. Porter does ignore failed commands when the mixin doesn't support a called command, but after I updated that bit of code to use spans and strutured logging, I accidentally caused porter to always print out if the command failed, even when the verbosity is greater than debug. This fixes failed mixin commands to only print the error when verbosity is debug and I've added a CLI test to validate that we print when expected to avoid future regressions in this area. Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
1 parent 0643d29 commit 5700952

File tree

3 files changed

+46
-2
lines changed

3 files changed

+46
-2
lines changed

cmd/testmixin/main.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ func main() {
2525
// This is a mixin that helps us test out our schema command
2626
fmt.Println(schema)
2727
case "lint":
28-
fmt.Println("[]")
28+
// The test mixin does not implement lint
29+
fmt.Fprintln(os.Stderr, `unknown command "lint" for "testmixin"`)
30+
os.Exit(1)
2931
case "build":
3032
fmt.Println("# testmixin")
3133
case "run":

pkg/pkgmgmt/client/runner.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,11 @@ func (r *Runner) Run(ctx context.Context, commandOpts pkgmgmt.CommandOptions) er
9999
err = cmd.Wait()
100100
if err != nil {
101101
// Include stderr in the error, otherwise it just includes the exit code
102-
return span.Error(fmt.Errorf("package command failed %s\n%s", prettyCmd, cmdStderr))
102+
err = fmt.Errorf("package command failed %s\n%s", prettyCmd, cmdStderr)
103+
// Do not flag this as an error in the logs because we often call mixins to see if they support a command
104+
// and if they don't it's not an error, e.g. not all mixins support lint or schema
105+
span.Debugf(err.Error())
106+
return err
103107
}
104108

105109
return nil

tests/integration/lint_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//go:build integration
2+
3+
package integration
4+
5+
import (
6+
"path/filepath"
7+
"testing"
8+
9+
"get.porter.sh/porter/tests/tester"
10+
"github.com/carolynvs/magex/shx"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestLint(t *testing.T) {
15+
test, err := tester.NewTest(t)
16+
defer test.Close()
17+
require.NoError(t, err, "test setup failed")
18+
19+
// When a mixin doesn't support lint, we should not print that to the console unless we are in debug mode
20+
_, output, _ := test.RunPorterWith(func(cmd *shx.PreparedCommand) {
21+
cmd.Args("lint")
22+
// mybuns uses the testmixin which doesn't support lint
23+
cmd.In(filepath.Join(test.RepoRoot, "tests/testdata/mybuns"))
24+
// change verbosity to debug so that we see the error
25+
cmd.Env("PORTER_VERBOSITY=debug")
26+
})
27+
require.Contains(t, output, "unknown command", "an unsupported mixin command should print to the console in debug")
28+
29+
_, output, _ = test.RunPorterWith(func(cmd *shx.PreparedCommand) {
30+
cmd.Args("lint")
31+
// mybuns uses the testmixin which doesn't support lint
32+
cmd.In(filepath.Join(test.RepoRoot, "tests/testdata/mybuns"))
33+
// errors are printed at the debug level to bump it up to info
34+
cmd.Env("PORTER_VERBOSITY=info")
35+
})
36+
require.NotContains(t, output, "unknown command", "an unsupported mixin command should not be printed to the console in info")
37+
38+
}

0 commit comments

Comments
 (0)