fix(query.go): actually propogate mixin runErr (#989)

Signed-off-by: Vaughn Dice <[email protected]>

pkg/build/dockerfile-generator_test.go

@@ -297,3 +297,22 @@ COPY mybin /cnab/app/
 	// however, the # PORTER_MIXINS token should be removed
 	assert.Equal(t, wantLines, gotlines)
 }
+
+func TestPorter_buildMixinsSection_mixinErr(t *testing.T) {
+	c := config.NewTestConfig(t)
+	tmpl := templates.NewTemplates()
+	configTpl, err := tmpl.GetManifest()
+	require.Nil(t, err)
+	c.TestContext.AddTestFileContents(configTpl, config.Name)
+
+	m, err := manifest.LoadManifestFrom(c.Context, config.Name)
+	require.NoError(t, err, "could not load manifest")
+
+	m.Mixins = []manifest.MixinDeclaration{{Name: "exec"}}
+
+	mp := mixin.NewTestMixinProvider()
+	mp.ReturnBuildError = true
+	g := NewDockerfileGenerator(c.Config, m, tmpl, mp)
+	_, err = g.buildMixinsSection()
+	require.EqualError(t, err, "1 error occurred:\n\t* error encountered from mixin \"exec\": encountered build error\n\n")
+}

pkg/mixin/helpers.go

@@ -2,6 +2,7 @@ package mixin
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 
@@ -16,6 +17,10 @@ type TestMixinProvider struct {
 	// LintResults allows you to provide linter.Results for your unit tests.
 	// It isn't of type linter.Results directly to avoid package cycles
 	LintResults interface{}
+
+	// ReturnBuildError will force the TestMixinProvider to return a build error
+	// if set to true
+	ReturnBuildError bool
 }
 
 // NewTestMixinProvider helps us test Porter.Mixins in our unit tests without actually hitting any real plugins on the file system.
@@ -38,21 +43,25 @@ func NewTestMixinProvider() *TestMixinProvider {
 		},
 	}
 
-	provider.RunAssertions = []func(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions){
+	provider.RunAssertions = []func(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions) error{
 		provider.PrintExecOutput,
 	}
 
 	return &provider
 }
 
-func (p *TestMixinProvider) PrintExecOutput(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions) {
+func (p *TestMixinProvider) PrintExecOutput(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions) error {
 	switch commandOpts.Command {
 	case "build":
+		if p.ReturnBuildError {
+			return errors.New("encountered build error")
+		}
 		fmt.Fprintln(pkgContext.Out, "# exec mixin has no buildtime dependencies")
 	case "lint":
 		b, _ := json.Marshal(p.LintResults)
 		fmt.Fprintln(pkgContext.Out, string(b))
 	}
+	return nil
 }
 
 func (p *TestMixinProvider) GetSchema(name string) (string, error) {

pkg/mixin/query/query.go

@@ -110,20 +110,21 @@ func (q *MixinQuery) Execute(cmd string, inputGenerator MixinInputGenerator) (ma
 		if response.runErr == nil {
 			results[response.mixinName] = response.output
 		} else {
-			runErr = multierror.Append(runErr, response.runErr)
+			runErr = multierror.Append(runErr,
+				errors.Wrapf(response.runErr, "error encountered from mixin %q", response.mixinName))
 		}
 	}
 
 	if runErr != nil {
 		if q.RequireAllMixinResponses {
-			return nil, err
+			return nil, runErr
 		}
 
 		// This is a debug because we expect not all mixins to implement some
 		// optional commands, like lint and don't want to print their error
 		// message when we query them with a command they don't support.
 		if q.Debug {
-			fmt.Fprintln(q.Err, errors.Wrap(err, "not all mixins responded successfully"))
+			fmt.Fprintln(q.Err, errors.Wrap(runErr, "not all mixins responded successfully"))
 		}
 	}
 

pkg/pkgmgmt/client/helpers.go

@@ -16,7 +16,7 @@ var _ pkgmgmt.PackageManager = &TestPackageManager{}
 type TestPackageManager struct {
 	PkgType       string
 	Packages      []pkgmgmt.PackageMetadata
-	RunAssertions []func(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions)
+	RunAssertions []func(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions) error
 }
 
 func (p *TestPackageManager) List() ([]string, error) {
@@ -52,7 +52,10 @@ func (p *TestPackageManager) Uninstall(o pkgmgmt.UninstallOptions) error {
 
 func (p *TestPackageManager) Run(pkgContext *context.Context, name string, commandOpts pkgmgmt.CommandOptions) error {
 	for _, assert := range p.RunAssertions {
-		assert(pkgContext, name, commandOpts)
+		err := assert(pkgContext, name, commandOpts)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }

pkg/porter/run_test.go

@@ -6,11 +6,10 @@ import (
 
 	"get.porter.sh/porter/pkg/mixin"
 
-	"get.porter.sh/porter/pkg/pkgmgmt"
-
 	"get.porter.sh/porter/pkg/config"
 	"get.porter.sh/porter/pkg/context"
 	"get.porter.sh/porter/pkg/manifest"
+	"get.porter.sh/porter/pkg/pkgmgmt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -20,10 +19,11 @@ func TestPorter_Run(t *testing.T) {
 
 	// Mock the mixin test runner and verify that we are calling runtime mixins, e.g. exec-runtime and not exec
 	mp := p.Mixins.(*mixin.TestMixinProvider)
-	mp.RunAssertions = append(mp.RunAssertions, func(mixinCxt *context.Context, mixinName string, commandOpts pkgmgmt.CommandOptions) {
+	mp.RunAssertions = append(mp.RunAssertions, func(mixinCxt *context.Context, mixinName string, commandOpts pkgmgmt.CommandOptions) error {
 		assert.Equal(t, "exec", mixinName, "expected to call the exec mixin")
 		assert.True(t, commandOpts.Runtime, "the mixin command should be executed in runtime mode")
 		assert.Equal(t, "install", commandOpts.Command, "should have executed the mixin's install command")
+		return nil
 	})
 	p.TestConfig.TestContext.AddTestFile("testdata/bundle.json", "/cnab/bundle.json")
 	p.TestConfig.TestContext.AddTestFile("testdata/porter.yaml", "porter.yaml")