Skip to content

Commit

Permalink
Pass T explicitly to all methods (#102)
Browse files Browse the repository at this point in the history
# ⚠️ Breaking change ⚠️ 

Don't capture T within the PulumiTest structure as this breaks various
aspects of how Go's testing is designed:
1. Sub-tests are given their own `T` instance so if we create a copy of
a `pulumitest` instance for a sub-test, we don't want logs reported to
the parent.
2. Logs are incorrectly reported back to the wrong line number. The logs
are tracked back to the construction of the `pulumitest` rather than the
line of the operation that failed.

Fixes #104 

## Why pass in `T`?

1. Logging: We can automatically log significant context in case of
failure.
2. Clean-up: When creating directories and stacks, we automatically
register them for deletion after the test has passed.
3. Assertions: We automatically assert that operations are successful. 

## Solution

- Pass T as the first parameter for any method which does asserts
internally as is the pattern in Go.
- Write an automated migration to update all existing usages of the
library.

### Migration Script

We can fix pretty much all usages using `gofmt`.

```bash
gofmt -r 'a.Convert(b) -> a.Convert(t, b)' -w ./**/*_test.go
gofmt -r 'a.CopyTo(b) -> a.CopyTo(t, b)' -w ./**/*_test.go
gofmt -r 'a.CopyToTempDir() -> a.CopyToTempDir(t)' -w ./**/*_test.go
gofmt -r 'a.Destroy() -> a.Destroy(t)' -w ./**/*_test.go
gofmt -r 'a.ExportStack() -> a.ExportStack(t)' -w ./**/*_test.go
gofmt -r 'a.GrpcLog() -> a.GrpcLog(t)' -w ./**/*_test.go
gofmt -r 'a.ClearGrpcLog() -> a.ClearGrpcLog(t)' -w ./**/*_test.go
gofmt -r 'a.Import(b, c, d, e, f, g) -> a.Import(t, b, c, d, e, f, g)' -w ./**/*_test.go
gofmt -r 'a.Import(b, c, d, e, f) -> a.Import(t, b, c, d, e, f)' -w ./**/*_test.go
gofmt -r 'a.Import(b, c, d, e) -> a.Import(t, b, c, d, e)' -w ./**/*_test.go
gofmt -r 'a.Import(b, c, d) -> a.Import(t, b, c, d)' -w ./**/*_test.go
gofmt -r 'a.Import(b, c) -> a.Import(t, b, c)' -w ./**/*_test.go
gofmt -r 'a.Import(b) -> a.Import(t, b)' -w ./**/*_test.go
gofmt -r 'a.ImportStack(b) -> a.ImportStack(t, b)' -w ./**/*_test.go
gofmt -r 'a.Install() -> a.Install(t)' -w ./**/*_test.go
gofmt -r 'a.InstallStack(b) -> a.InstallStack(t, b)' -w ./**/*_test.go
gofmt -r 'a.Preview() -> a.Preview(t)' -w ./**/*_test.go
gofmt -r 'a.Refresh() -> a.Refresh(t)' -w ./**/*_test.go
gofmt -r 'a.SetConfig(b, c) -> a.SetConfig(t, b, c)' -w ./**/*_test.go
gofmt -r 'a.Up() -> a.Up(t)' -w ./**/*_test.go
gofmt -r 'a.UpdateSource(b) -> a.UpdateSource(t, b)' -w ./**/*_test.go
gofmt -r 'a.NewStack(b) -> a.NewStack(t, b)' -w ./**/*_test.go
gofmt -r 'a.Run(b) -> a.Run(t, b)' -w ./**/*_test.go
```

- It's a little messy but correctly migrates all usages within this
package.
- ~`gofmt` seems unable to migrate the `pt.Run()` because it can't match
inline functions~. Re-testing shows this works.
- The `Import` migration isn't idempotent due to the variable number of
arguments so should be run once manually when upgrading to the new
version and the changes committed.
- This doesn't handle any instances of passing additional functional
argument, but I doubt we use many of these in practice.
  • Loading branch information
danielrbradley authored Aug 22, 2024
1 parent af0614e commit be10035
Show file tree
Hide file tree
Showing 31 changed files with 216 additions and 233 deletions.
12 changes: 6 additions & 6 deletions previewProviderUpgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ import (
// Uses a default cache directory of "testdata/recorded/TestProviderUpgrade/{programName}/{baselineVersion}".
func PreviewProviderUpgrade(t pulumitest.PT, pulumiTest *pulumitest.PulumiTest, providerName string, baselineVersion string, opts ...optproviderupgrade.PreviewProviderUpgradeOpt) auto.PreviewResult {
t.Helper()
previewTest := pulumiTest.CopyToTempDir(opttest.NewStackOptions(optnewstack.DisableAutoDestroy()))
previewTest := pulumiTest.CopyToTempDir(t, opttest.NewStackOptions(optnewstack.DisableAutoDestroy()))
options := optproviderupgrade.Defaults()
for _, opt := range opts {
opt.Apply(&options)
}
programName := filepath.Base(pulumiTest.Source())
cacheDir := getCacheDir(options, programName, baselineVersion)
previewTest.Run(
previewTest.Run(t,
func(test *pulumitest.PulumiTest) {
t.Helper()
test.Up()
grptLog := test.GrpcLog()
test.Up(t)
grptLog := test.GrpcLog(t)
grpcLogPath := filepath.Join(cacheDir, "grpc.json")
t.Log(fmt.Sprintf("writing grpc log to %s", grpcLogPath))
grptLog.WriteTo(grpcLogPath)
Expand All @@ -41,9 +41,9 @@ func PreviewProviderUpgrade(t pulumitest.PT, pulumiTest *pulumitest.PulumiTest,
)

if options.NewSourcePath != "" {
previewTest.UpdateSource(options.NewSourcePath)
previewTest.UpdateSource(t, options.NewSourcePath)
}
return previewTest.Preview()
return previewTest.Preview(t)
}

func baselineProviderOpt(options optproviderupgrade.PreviewProviderUpgradeOptions, providerName string, baselineVersion string) opttest.Option {
Expand Down
2 changes: 1 addition & 1 deletion providers/providerInterceptProxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,6 @@ func TestProviderInterceptProxy(t *testing.T) {
filepath.Join("..", "pulumitest", "testdata", "yaml_azure"),
opttest.AttachProvider("azure-native", interceptedFactory))

test.Preview()
test.Preview(t)
assert.True(t, didAttach, "expected Attach to be called in proxy")
}
6 changes: 3 additions & 3 deletions providers/providerMock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestProviderMock(t *testing.T) {
test := pulumitest.NewPulumiTest(t, source,
opttest.AttachProvider("gcp",
providers.ProviderMockFactory(providers.ProviderMocks{})))
test.Preview()
test.Preview(t)
})

t.Run("with mocks", func(t *testing.T) {
Expand Down Expand Up @@ -60,8 +60,8 @@ func TestProviderMock(t *testing.T) {
return &emptypb.Empty{}, nil
},
})))
test.Preview()
test.Up()
test.Preview(t)
test.Up(t)
assert.True(t, attached, "expected Attach to be called in mock")
assert.True(t, configured, "expected Configure to be called in mock")
assert.True(t, checkedConfig, "expected CheckConfig to be called in mock")
Expand Down
15 changes: 7 additions & 8 deletions pulumitest/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,23 @@ type ConvertResult struct {

// Convert a program to a given language.
// It returns a new PulumiTest instance for the converted program which will be outputted into a temporary directory.
func (a *PulumiTest) Convert(language string, opts ...opttest.Option) ConvertResult {
a.t.Helper()
func (a *PulumiTest) Convert(t PT, language string, opts ...opttest.Option) ConvertResult {
t.Helper()

tempDir := a.t.TempDir()
tempDir := t.TempDir()
base := filepath.Base(a.workingDir)
targetDir := filepath.Join(tempDir, fmt.Sprintf("%s-%s", base, language))
err := os.Mkdir(targetDir, 0755)
if err != nil {
a.fatal(err)
ptFatal(t, err)
}

a.logf("converting to %s", language)
ptLogF(t, "converting to %s", language)
cmd := exec.Command("pulumi", "convert", "--language", language, "--generate-only", "--out", targetDir)
cmd.Dir = a.workingDir
out, err := cmd.CombinedOutput()
if err != nil {
a.fatalf("failed to convert directory: %s\n%s", err, out)
ptFatalF(t, "failed to convert directory: %s\n%s", err, out)
}

options := a.options.Copy()
Expand All @@ -43,12 +43,11 @@ func (a *PulumiTest) Convert(language string, opts ...opttest.Option) ConvertRes
}

convertedTest := &PulumiTest{
t: a.t,
ctx: a.ctx,
workingDir: targetDir,
options: options,
}
pulumiTestInit(convertedTest, options)
pulumiTestInit(t, convertedTest, options)
return ConvertResult{
PulumiTest: convertedTest,
Output: string(out),
Expand Down
19 changes: 9 additions & 10 deletions pulumitest/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,41 @@ import (
// CopyToTempDir copies the program to a temporary directory.
// It returns a new PulumiTest instance for the copied program.
// This is used to avoid temporary files being written to the source directory.
func (a *PulumiTest) CopyToTempDir(opts ...opttest.Option) *PulumiTest {
a.t.Helper()
tempDir := tempDirWithoutCleanupOnFailedTest(a.t, "programDir")
func (a *PulumiTest) CopyToTempDir(t PT, opts ...opttest.Option) *PulumiTest {
t.Helper()
tempDir := tempDirWithoutCleanupOnFailedTest(t, "programDir")

// Maintain the directory name in the temp dir as this might be used for stack naming.
sourceBase := filepath.Base(a.workingDir)
destination := filepath.Join(tempDir, sourceBase)
err := os.Mkdir(destination, 0755)
if err != nil {
a.fatal(err)
ptFatal(t, err)
}

return a.CopyTo(destination, opts...)
return a.CopyTo(t, destination, opts...)
}

// CopyTo copies the program to the specified directory.
// It returns a new PulumiTest instance for the copied program.
func (a *PulumiTest) CopyTo(dir string, opts ...opttest.Option) *PulumiTest {
a.t.Helper()
func (a *PulumiTest) CopyTo(t PT, dir string, opts ...opttest.Option) *PulumiTest {
t.Helper()

err := copyDirectory(a.workingDir, dir)
if err != nil {
a.fatal(err)
ptFatal(t, err)
}

options := a.options.Copy()
for _, opt := range opts {
opt.Apply(options)
}
newTest := &PulumiTest{
t: a.t,
ctx: a.ctx,
workingDir: dir,
options: options,
}
pulumiTestInit(newTest, options)
pulumiTestInit(t, newTest, options)
return newTest
}

Expand Down
12 changes: 6 additions & 6 deletions pulumitest/destroy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ import (
)

// Up deploys the current stack.
func (a *PulumiTest) Destroy(opts ...optdestroy.Option) auto.DestroyResult {
a.t.Helper()
func (a *PulumiTest) Destroy(t PT, opts ...optdestroy.Option) auto.DestroyResult {
t.Helper()

a.t.Log("destroying")
t.Log("destroying")
if a.currentStack == nil {
a.fatal("no current stack")
ptFatal(t, "no current stack")
}
if !a.options.DisableGrpcLog {
a.ClearGrpcLog()
a.ClearGrpcLog(t)
}
result, err := a.currentStack.Destroy(a.ctx, opts...)
if err != nil {
a.fatalf("failed to destroy: %s", err)
ptFatalF(t, "failed to destroy: %s", err)
}
return result
}
4 changes: 2 additions & 2 deletions pulumitest/destroy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ import (
func TestDestroy(t *testing.T) {
t.Parallel()
test := pulumitest.NewPulumiTest(t, "testdata/yaml_program")
test.Up()
test.Destroy()
test.Up(t)
test.Destroy(t)
}
23 changes: 11 additions & 12 deletions pulumitest/execCmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import (
)

type cmdOutput struct {
Args []string
Stdout string
Stderr string
Args []string
Stdout string
Stderr string
ReturnCode int
}

func (a *PulumiTest) execCmd(args ...string) cmdOutput {
a.t.Helper()
func (a *PulumiTest) execCmd(t PT, args ...string) cmdOutput {
t.Helper()
workspace := a.CurrentStack().Workspace()
ctx := context.Background()
workdir := workspace.WorkDir()
Expand All @@ -26,16 +26,15 @@ func (a *PulumiTest) execCmd(args ...string) cmdOutput {

s1, s2, code, err := workspace.PulumiCommand().Run(ctx, workdir, stdin, nil, nil, env, args...)
if err != nil {
a.logf(s1)
a.logf(s2)
a.fatalf("Failed to run command %v: %v", args, err)
ptLogF(t, s1)
ptLogF(t, s2)
ptFatalF(t, "Failed to run command %v: %v", args, err)
}

return cmdOutput{
Args: args,
Stdout: s1,
Stderr: s2,
Args: args,
Stdout: s1,
Stderr: s2,
ReturnCode: code,
}
}

10 changes: 5 additions & 5 deletions pulumitest/exportStack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ import (
)

// ExportStack exports the current stack state.
func (a *PulumiTest) ExportStack() apitype.UntypedDeployment {
a.t.Helper()
func (a *PulumiTest) ExportStack(t PT) apitype.UntypedDeployment {
t.Helper()

a.t.Log("exporting stack")
t.Log("exporting stack")
if a.currentStack == nil {
a.fatal("no current stack")
ptFatal(t, "no current stack")
}
out, err := a.currentStack.Workspace().ExportStack(a.Context(), a.currentStack.Name())
if err != nil {
a.fatalf("failed to export stack: %s", err)
ptFatalF(t, "failed to export stack: %s", err)
}
return out
}
15 changes: 8 additions & 7 deletions pulumitest/grpcLog.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,34 +7,35 @@ import (
)

// GrpcLog reads the gRPC log for the current stack based on the PULUMI_DEBUG_GRPC env var.
func (pt *PulumiTest) GrpcLog() *grpclog.GrpcLog {
pt.t.Helper()
func (pt *PulumiTest) GrpcLog(t PT) *grpclog.GrpcLog {
t.Helper()

if pt.currentStack == nil {
pt.t.Log("can't read gRPC log: no current stack")
t.Log("can't read gRPC log: no current stack")
return nil
}

env := pt.CurrentStack().Workspace().GetEnvVars()
if env == nil || env["PULUMI_DEBUG_GRPC"] == "" {
pt.t.Log("can't read gRPC log: PULUMI_DEBUG_GRPC env var not set")
t.Log("can't read gRPC log: PULUMI_DEBUG_GRPC env var not set")
return nil
}

log, err := grpclog.LoadLog(env["PULUMI_DEBUG_GRPC"])
if err != nil {
pt.fatalf("failed to load grpc log: %s", err)
ptFatalF(t, "failed to load grpc log: %s", err)
}
return log
}

// ClearGrpcLog clears the gRPC log for the current stack based on the PULUMI_DEBUG_GRPC env var.
func (pt *PulumiTest) ClearGrpcLog() {
func (pt *PulumiTest) ClearGrpcLog(t PT) {
t.Helper()
env := pt.CurrentStack().Workspace().GetEnvVars()
if env == nil || env["PULUMI_DEBUG_GRPC"] == "" {
return
}
if err := os.RemoveAll(env["PULUMI_DEBUG_GRPC"]); err != nil {
pt.fatalf("failed to clear gRPC log: %s", err)
ptFatalF(t, "failed to clear gRPC log: %s", err)
}
}
4 changes: 2 additions & 2 deletions pulumitest/grpcLog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
func TestGrpc(t *testing.T) {
t.Parallel()
test := pulumitest.NewPulumiTest(t, "testdata/yaml_program")
test.Up()
log := test.GrpcLog()
test.Up(t)
log := test.GrpcLog(t)
assert.NotEmpty(t, log)
creates, err := log.Creates()
assert.NoError(t, err)
Expand Down
12 changes: 5 additions & 7 deletions pulumitest/import.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
package pulumitest

func (a *PulumiTest) Import(
resourceType, resourceName, resourceID string, providerUrn string, args ...string,
) cmdOutput {
a.t.Helper()
func (a *PulumiTest) Import(t PT, resourceType, resourceName, resourceID string, providerUrn string, args ...string) cmdOutput {
t.Helper()
arguments := []string{
"import", resourceType, resourceName, resourceID, "--yes", "--protect=false", "-s", a.CurrentStack().Name(),
}
if providerUrn != "" {
arguments = append(arguments, "--provider="+providerUrn)
}
arguments = append(arguments, args...)
ret := a.execCmd(arguments...)
ret := a.execCmd(t, arguments...)
if ret.ReturnCode != 0 {
a.log(ret.Stdout)
a.fatalf("failed to import resource %s: %s", resourceName, ret.Stderr)
t.Log(ret.Stdout)
ptFatalF(t, "failed to import resource %s: %s", resourceName, ret.Stderr)
}

return ret
Expand Down
10 changes: 5 additions & 5 deletions pulumitest/importStack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import (
)

// ImportStack imports the given stack state into the test's current stack.
func (a *PulumiTest) ImportStack(source apitype.UntypedDeployment) {
a.t.Helper()
func (a *PulumiTest) ImportStack(t PT, source apitype.UntypedDeployment) {
t.Helper()

a.t.Log("importing stack")
t.Log("importing stack")
if a.currentStack == nil {
a.fatal("no current stack")
ptFatal(t, "no current stack")
}
err := a.currentStack.Workspace().ImportStack(a.Context(), a.currentStack.Name(), source)
if err != nil {
a.fatalf("failed to import stack: %s", err)
ptFatalF(t, "failed to import stack: %s", err)
}
}
8 changes: 4 additions & 4 deletions pulumitest/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ func TestImport(t *testing.T) {
t.Parallel()
test := pulumitest.NewPulumiTest(t, "testdata/yaml_empty")

res := test.Import("random:index/randomString:RandomString", "str", "importedString", "")
res := test.Import(t, "random:index/randomString:RandomString", "str", "importedString", "")

// Assert on the generated YAML containing a resource definition
require.Contains(t, res.Stdout, "type: random:RandomString")

// Assert on the stack containing the resource state
stack := test.ExportStack()
stack := test.ExportStack(t)
data, err := stack.Deployment.MarshalJSON()
require.NoError(t, err)
var stateMap map[string]interface{}
Expand All @@ -47,7 +47,7 @@ func TestImportWithArgs(t *testing.T) {
test := pulumitest.NewPulumiTest(t, "testdata/yaml_empty")

outFile := filepath.Join(test.CurrentStack().Workspace().WorkDir(), "out.yaml")
res := test.Import("random:index/randomString:RandomString", "str", "importedString", "", "--out", outFile)
res := test.Import(t, "random:index/randomString:RandomString", "str", "importedString", "", "--out", outFile)

assert.Equal(t, []string{
"import",
Expand All @@ -62,7 +62,7 @@ func TestImportWithArgs(t *testing.T) {
assert.Contains(t, string(contents), "type: random:RandomString")

// Assert on the stack containing the resource state
stack := test.ExportStack()
stack := test.ExportStack(t)
data, err := stack.Deployment.MarshalJSON()
require.NoError(t, err)
var stateMap map[string]interface{}
Expand Down
Loading

0 comments on commit be10035

Please sign in to comment.