Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Commit

Permalink
(GH-342) Genericize build cmd
Browse files Browse the repository at this point in the history
Prior to this commit, the cobra `build` command used package variables
and instantiated a PCT-specific `build` command in the `CreateCommand()`
function. Any other cobra applications wanting to use the now-generic
functionality from the public `build` package would need to reimplement
this command wholly themselves as it hard-codes to PCT's packaging
paradigm (i.e. it references "templates", expects a `pct-config.yml`,
and instantiates the private PCT `ConfigProcessor`).

This commit genericizes the `build` command by:

1. Replacing the package variables with a `BuildCommand` struct,
   declaring a `BuildCommandI` interface, and updating the functions
   (`CreateCommand()`, `preExecute()`, and `execute()`) to be attached
   to the new `BuildCommand` struct.
2. Replacing references in the `build` command's reference doc help text
   and error/info messaging, updating default language to "project" and
   extending the `BuildCommand` struct to include defining a
   `ProjectType` to clarify those docs/messages (i.e. "Builds a package
   from the template project" for PCT where `ProjectType` is set in the
   struct as `template`).
3. Moving the instantiation of the struct to `main` where it  defines
   the `ProjectType` as `template` and instantiates the `build.Builder`
   struct, passing in the private `PctConfigProcessor`.

During testing for this change, I noticed that the unit tests for the
`build` command did not mock the `build.Builder`; this was because the
`CreateCommand()` function instantiates that struct inside itself.
Therefore, the unit tests were _actually_ trying to build packages.

This commit reworks and simplifies those tests so that they use a new
mock `Builder` instead of functionally (and without documentation)
running integration tests for the `build` package.
  • Loading branch information
michaeltlombardi committed Feb 14, 2022
1 parent 33c4718 commit 917ae7c
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 109 deletions.
6 changes: 3 additions & 3 deletions acceptance/build/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func Test_PctBuild_Outputs_TarGz(t *testing.T) {

expectedOutputFilePath := filepath.Join(wd, fmt.Sprintf("%v.tar.gz", templateName))

assert.Contains(t, stdout, fmt.Sprintf("Template output to %v", expectedOutputFilePath))
assert.Contains(t, stdout, fmt.Sprintf("Packaged template output to %v", expectedOutputFilePath))
assert.Equal(t, "", stderr)
assert.Equal(t, 0, exitCode)
assert.FileExists(t, expectedOutputFilePath)
Expand All @@ -47,7 +47,7 @@ func Test_PctBuild_With_NoTargetDir_Outputs_TarGz(t *testing.T) {

expectedOutputFilePath := filepath.Join(wd, "pkg", fmt.Sprintf("%v.tar.gz", templateName))

assert.Contains(t, stdout, fmt.Sprintf("Template output to %v", expectedOutputFilePath))
assert.Contains(t, stdout, fmt.Sprintf("Packaged template output to %v", expectedOutputFilePath))
assert.Equal(t, "", stderr)
assert.Equal(t, 0, exitCode)
assert.FileExists(t, expectedOutputFilePath)
Expand All @@ -65,7 +65,7 @@ func Test_PctBuild_With_EmptySourceDir_Errors(t *testing.T) {
cmd := fmt.Sprintf("build --sourcedir %v", templateDir)
stdout, stderr, exitCode := testutils.RunAppCommand(cmd, "")

assert.Contains(t, stdout, fmt.Sprintf("No template directory at %v", templateDir))
assert.Contains(t, stdout, fmt.Sprintf("No project directory at %v", templateDir))
assert.Equal(t, "exit status 1", stderr)
assert.Equal(t, 1, exitCode)
}
Expand Down
62 changes: 27 additions & 35 deletions cmd/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,71 +5,63 @@ import (
"os"
"path/filepath"

"github.com/puppetlabs/pdkgo/internal/pkg/pct_config_processor"
"github.com/puppetlabs/pdkgo/pkg/build"
"github.com/puppetlabs/pdkgo/pkg/gzip"
"github.com/puppetlabs/pdkgo/pkg/tar"
"github.com/rs/zerolog/log"
"github.com/spf13/afero"
"github.com/spf13/cobra"
)

var (
sourceDir string
targetDir string
builder *build.Builder
)
type BuildCommand struct {
SourceDir string
TargetDir string
ProjectType string
Builder build.BuilderI
}

type BuildCommandI interface {
CreateCommand() *cobra.Command
}

func CreateCommand() *cobra.Command {
func (bc *BuildCommand) CreateCommand() *cobra.Command {
tmp := &cobra.Command{
Use: "build [flags]",
Short: "Builds a package from the template",
Long: `Builds a package from the template. Assumes the current working directory is the template you wish to package`,
PreRunE: preExecute,
RunE: execute,
Short: fmt.Sprintf("Builds a package from the %s project", bc.ProjectType),
Long: fmt.Sprintf("Builds a package from the %s project. Assumes the current working directory is the template you wish to package", bc.ProjectType),
PreRunE: bc.preExecute,
RunE: bc.execute,
}

tmp.Flags().StringVar(&sourceDir, "sourcedir", "", "The template directory you wish to package up")
tmp.Flags().StringVar(&targetDir, "targetdir", "", "The target directory where you want the packaged template to be output to")

fs := afero.NewOsFs() // configure afero to use real filesystem
builder = &build.Builder{
Tar: &tar.Tar{AFS: &afero.Afero{Fs: fs}},
Gzip: &gzip.Gzip{AFS: &afero.Afero{Fs: fs}},
AFS: &afero.Afero{Fs: fs},
ConfigProcessor: &pct_config_processor.PctConfigProcessor{AFS: &afero.Afero{Fs: fs}},
ConfigFile: "pct-config.yml",
}
tmp.Flags().StringVar(&bc.SourceDir, "sourcedir", "", fmt.Sprintf("The %s project directory you wish to package up", bc.ProjectType))
tmp.Flags().StringVar(&bc.TargetDir, "targetdir", "", fmt.Sprintf("The target directory where you want the packaged %s project to be output to", bc.ProjectType))

return tmp
}

func preExecute(cmd *cobra.Command, args []string) error {
func (bc *BuildCommand) preExecute(cmd *cobra.Command, args []string) error {

wd, err := os.Getwd()
log.Trace().Msgf("WD: %v", wd)

if (sourceDir == "" || targetDir == "") && err != nil {
if (bc.SourceDir == "" || bc.TargetDir == "") && err != nil {
return err
}

if sourceDir == "" {
sourceDir = wd
if bc.SourceDir == "" {
bc.SourceDir = wd
}

if targetDir == "" {
targetDir = filepath.Join(wd, "pkg")
if bc.TargetDir == "" {
bc.TargetDir = filepath.Join(wd, "pkg")
}

return nil
}

func execute(cmd *cobra.Command, args []string) error {
gzipArchiveFilePath, err := builder.Build(sourceDir, targetDir)
func (bc *BuildCommand) execute(cmd *cobra.Command, args []string) error {
gzipArchiveFilePath, err := bc.Builder.Build(bc.SourceDir, bc.TargetDir)

if err != nil {
return fmt.Errorf("`sourcedir` is not a valid template: %s", err.Error())
return fmt.Errorf("`sourcedir` is not a valid %s project: %s", bc.ProjectType, err.Error())
}
log.Info().Msgf("Template output to %v", gzipArchiveFilePath)
log.Info().Msgf("Packaged %s output to %v", bc.ProjectType, gzipArchiveFilePath)
return nil
}
117 changes: 48 additions & 69 deletions cmd/build/build_test.go
Original file line number Diff line number Diff line change
@@ -1,105 +1,84 @@
package build
package build_test

import (
"bytes"
"io/ioutil"
"os"
"path/filepath"
"regexp"
"testing"

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

func nullFunction(cmd *cobra.Command, args []string) error {
return nil
}
"github.com/puppetlabs/pdkgo/cmd/build"
"github.com/puppetlabs/pdkgo/pkg/mock"
)

func TestCreatebuildCommand(t *testing.T) {
func TestCreateBuildCommand(t *testing.T) {
wd, _ := os.Getwd()
defaultSourceDir := wd
defaultTargetDir := filepath.Join(wd, "pkg")

tests := []struct {
name string
args []string
returnCode int
out string
wantCmd *cobra.Command
wantErr bool
f func(cmd *cobra.Command, args []string) error
expSrcDir string
expTargDir string
name string
args []string
expectedSourceDir string
expectedTargetDir string
expectedErrorMatch string
}{
{
name: "executes without error for valid flag",
args: []string{"build"},
f: nullFunction,
out: "",
wantErr: false,
expSrcDir: defaultSourceDir,
expTargDir: defaultTargetDir,
name: "executes without error when no flags passed",
args: []string{},
expectedSourceDir: defaultSourceDir,
expectedTargetDir: defaultTargetDir,
},
{
name: "executes with error for invalid flag",
args: []string{"--foo"},
f: nullFunction,
out: "unknown flag: --foo",
wantErr: true,
expSrcDir: "",
expTargDir: "",
name: "executes with error for invalid flag",
args: []string{"--foo"},
expectedErrorMatch: "unknown flag: --foo",
},
{
name: "uses sourcedir, targetdir when passed in",
args: []string{"build", "--sourcedir", "/path/to/template", "--targetdir", "/path/to/output"},
f: nullFunction,
out: "",
wantErr: false,
expSrcDir: "/path/to/template",
expTargDir: "/path/to/output",
name: "uses sourcedir, targetdir when passed in",
args: []string{"--sourcedir", "/path/to/template", "--targetdir", "/path/to/output"},
expectedSourceDir: "/path/to/template",
expectedTargetDir: "/path/to/output",
},
{
name: "Sets correct defaults if sourcedir and targetdir undefined",
args: []string{"build"},
f: nullFunction,
out: "",
wantErr: false,
expSrcDir: defaultSourceDir,
expTargDir: defaultTargetDir,
name: "Sets correct defaults if sourcedir and targetdir undefined",
args: []string{},
expectedSourceDir: defaultSourceDir,
expectedTargetDir: defaultTargetDir,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := CreateCommand()
b := bytes.NewBufferString("")
cmd.SetOut(b)
cmd.SetErr(b)
cmd.SetArgs(tt.args)
cmd.RunE = tt.f

err := cmd.Execute()
if (err != nil) != tt.wantErr {
t.Errorf("executeTestUnit() error = %v, wantErr %v", err, tt.wantErr)
return
cmd := build.BuildCommand{
ProjectType: "template",
Builder: &mock.Builder{
ProjectName: "my-project",
ExpectedSourceDir: tt.expectedSourceDir,
ExpectedTargetDir: tt.expectedTargetDir,
},
}
buildCmd := cmd.CreateCommand()

out, err := ioutil.ReadAll(b)
if err != nil {
t.Errorf("Failed to read stdout: %v", err)
return
}
b := bytes.NewBufferString("")
buildCmd.SetOut(b)
buildCmd.SetErr(b)

assert.Equal(t, tt.expSrcDir, sourceDir)
assert.Equal(t, tt.expTargDir, targetDir)
buildCmd.SetArgs(tt.args)
err := buildCmd.Execute()

output := string(out)
r := regexp.MustCompile(tt.out)
if !r.MatchString(output) {
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
return
if err != nil {
if tt.expectedErrorMatch == "" {
t.Errorf("Unexpected error when none wanted: %v", err)
return
} else {
out, _ := ioutil.ReadAll(b)
assert.Regexp(t, tt.expectedErrorMatch, string(out))
}
} else if tt.expectedErrorMatch != "" {
t.Errorf("Expected error '%s'but none raised", err)
}
})
}

}
15 changes: 13 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import (
"github.com/puppetlabs/pdkgo/internal/pkg/pct_config_processor"
"github.com/puppetlabs/pdkgo/pkg/exec_runner"

"github.com/puppetlabs/pdkgo/cmd/build"
cmd_build "github.com/puppetlabs/pdkgo/cmd/build"
"github.com/puppetlabs/pdkgo/cmd/completion"
"github.com/puppetlabs/pdkgo/cmd/explain"
cmd_install "github.com/puppetlabs/pdkgo/cmd/install"
"github.com/puppetlabs/pdkgo/cmd/new"
"github.com/puppetlabs/pdkgo/cmd/root"
appver "github.com/puppetlabs/pdkgo/cmd/version"
"github.com/puppetlabs/pdkgo/pkg/build"
"github.com/puppetlabs/pdkgo/pkg/gzip"
"github.com/puppetlabs/pdkgo/pkg/install"
"github.com/puppetlabs/pdkgo/pkg/tar"
Expand Down Expand Up @@ -59,7 +60,17 @@ func main() {
iofs := afero.IOFS{Fs: fs}

// build
rootCmd.AddCommand(build.CreateCommand())
buildCmd := cmd_build.BuildCommand{
ProjectType: "template",
Builder: &build.Builder{
Tar: &tar.Tar{AFS: &afero.Afero{Fs: fs}},
Gzip: &gzip.Gzip{AFS: &afero.Afero{Fs: fs}},
AFS: &afero.Afero{Fs: fs},
ConfigProcessor: &pct_config_processor.PctConfigProcessor{AFS: &afero.Afero{Fs: fs}},
ConfigFile: "pct-config.yml",
},
}
rootCmd.AddCommand(buildCmd.CreateCommand())

// install
installCmd := cmd_install.InstallCommand{
Expand Down
23 changes: 23 additions & 0 deletions pkg/mock/build.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package mock

import (
"fmt"
)

type Builder struct {
ProjectName string
ExpectedSourceDir string
ExpectedTargetDir string
}

func (b *Builder) Build(templatePath, targetDir string) (gzipArchiveFilePath string, err error) {
// if input isn't what's expected, raise an error
if templatePath != b.ExpectedSourceDir {
return "", fmt.Errorf("Expected source dir '%s' but got '%s'", b.ExpectedSourceDir, templatePath)
}
if targetDir != b.ExpectedTargetDir {
return "", fmt.Errorf("Expected source dir '%s' but got '%s'", b.ExpectedSourceDir, templatePath)
}
// If nothing goes wrong, return the path to the packaged project
return fmt.Sprintf("%s/my-project.tar.gz", targetDir), nil
}

0 comments on commit 917ae7c

Please sign in to comment.