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.

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

Support testing of configurations in JSON syntax. #722

Merged
merged 3 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions helper/resource/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,9 @@ type TestStep struct {
// Config a string of the configuration to give to Terraform. If this
// is set, then the TestCase will execute this step with the same logic
// as a `terraform apply`.
//
// JSON Configuration Syntax can be used and is assumed whenever Config
// contains valid JSON.
Config string

// Check is called after the Config is applied. Use this step to
Expand Down
36 changes: 26 additions & 10 deletions internal/plugintest/working_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package plugintest
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
Expand All @@ -15,8 +16,9 @@ import (
)

const (
ConfigFileName = "terraform_plugin_test.tf"
PlanFileName = "tfplan"
ConfigFileName = "terraform_plugin_test.tf"
ConfigFileNameJSON = ConfigFileName + ".json"
PlanFileName = "tfplan"
)

// WorkingDir represents a distinct working directory that can be used for
Expand All @@ -29,6 +31,10 @@ type WorkingDir struct {
// baseDir is the root of the working directory tree
baseDir string

// configFilename is the full filename where the latest configuration
// was stored; empty until SetConfig is called.
configFilename string

// baseArgs is arguments that should be appended to all commands
baseArgs []string

Expand Down Expand Up @@ -84,11 +90,20 @@ func (wd *WorkingDir) GetHelper() *Helper {
// Destroy to establish the configuration. Any previously-set configuration is
// discarded and any saved plan is cleared.
func (wd *WorkingDir) SetConfig(ctx context.Context, cfg string) error {
configFilename := filepath.Join(wd.baseDir, ConfigFileName)
err := ioutil.WriteFile(configFilename, []byte(cfg), 0700)
outFilename := filepath.Join(wd.baseDir, ConfigFileName)
rmFilename := filepath.Join(wd.baseDir, ConfigFileNameJSON)
bCfg := []byte(cfg)
if json.Valid(bCfg) {
outFilename, rmFilename = rmFilename, outFilename
}
if err := os.Remove(rmFilename); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("unable to remove %q: %w", rmFilename, err)
}
err := ioutil.WriteFile(outFilename, bCfg, 0700)
if err != nil {
return err
}
wd.configFilename = outFilename

var mismatch *tfexec.ErrVersionMismatch
err = wd.tf.SetDisablePluginTLS(true)
Expand Down Expand Up @@ -157,11 +172,16 @@ func (wd *WorkingDir) ClearPlan(ctx context.Context) error {
return nil
}

var errWorkingDirSetConfigNotCalled = fmt.Errorf("must call SetConfig before Init")

// Init runs "terraform init" for the given working directory, forcing Terraform
// to use the current version of the plugin under test.
func (wd *WorkingDir) Init(ctx context.Context) error {
if _, err := os.Stat(wd.configFilename()); err != nil {
return fmt.Errorf("must call SetConfig before Init")
if wd.configFilename == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

The original os.Stat() is likely safer here, in case configFilename isn't reset to "". 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the error message, I feel like the original os.Stat() is here as a test whether SetConfig has ever been called.

I re-added the call to os.Stat().

return errWorkingDirSetConfigNotCalled
}
if _, err := os.Stat(wd.configFilename); err != nil {
return errWorkingDirSetConfigNotCalled
}

logging.HelperResourceTrace(ctx, "Calling Terraform CLI init command")
Expand All @@ -173,10 +193,6 @@ func (wd *WorkingDir) Init(ctx context.Context) error {
return err
}

func (wd *WorkingDir) configFilename() string {
return filepath.Join(wd.baseDir, ConfigFileName)
}
Comment on lines -176 to -178
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, I think we should still keep this method around to automatically do the file joining:

func (wd *WorkingDir) configFilename() string {
	return filepath.Join(wd.baseDir, wd.configFilename)
}

And set without baseDir via:

if json.Valid(bCfg) {
	wd.configFilename = ConfigFileNameJSON
} else {
	wd.configFilename = ConfigFileName
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unlike WorkingDir.planFilename() -- which is "static" in the sense that it only depends on the Workdir.baseDir, configFilename is now "dynamic". It can be empty (SetConfig() not called yet) or one of the two cases (HCL/JSON).

When I tried your suggestion, configFilename() would:

  1. either have to return (string, error) (error in case no filename has been set up yet), or the caller would be responsible,
  2. or the caller would be responsible to check the filename field for emptiness.

The first option is of course the clean and correct one, but it felt cumbersome at the only place where this would be called (Init(), see below).

If you feel strongly, I can refactor it to the method that returns (string, error).


func (wd *WorkingDir) planFilename() string {
return filepath.Join(wd.baseDir, PlanFileName)
}
Expand Down
69 changes: 69 additions & 0 deletions internal/plugintest/working_dir_json_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package plugintest_test

import (
"context"
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

// TestJSONConfig verifies that TestStep.Config can contain JSON.
// This test also proves that when changing the HCL and JSON formats back and
// forth, the framework deletes the previous configuration file.
func TestJSONConfig(t *testing.T) {
providerFactories := map[string]func() (*schema.Provider, error){
"tst": func() (*schema.Provider, error) { return tstProvider(), nil }, //nolint:unparam // required signature
}
resource.Test(t, resource.TestCase{
IsUnitTest: true,
ProviderFactories: providerFactories,
Steps: []resource.TestStep{{
Config: `{"resource":{"tst_t":{"r1":{"s":"x1"}}}}`,
Check: resource.TestCheckResourceAttr("tst_t.r1", "s", "x1"),
}, {
Config: `resource "tst_t" "r1" { s = "x2" }`,
Check: resource.TestCheckResourceAttr("tst_t.r1", "s", "x2"),
}, {
Config: `{"resource":{"tst_t":{"r1":{"s":"x3"}}}}`,
Check: resource.TestCheckResourceAttr("tst_t.r1", "s", "x3"),
}},
})
}

func tstProvider() *schema.Provider {
return &schema.Provider{
ResourcesMap: map[string]*schema.Resource{
"tst_t": {
CreateContext: resourceTstTCreate,
ReadContext: resourceTstTRead,
UpdateContext: resourceTstTCreate, // Update is the same as Create
DeleteContext: resourceTstTDelete,
Schema: map[string]*schema.Schema{
"s": {
Type: schema.TypeString,
Required: true,
},
},
},
},
}
}

func resourceTstTCreate(ctx context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
d.SetId(d.Get("s").(string))
return nil
}

func resourceTstTRead(ctx context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
if err := d.Set("s", d.Id()); err != nil {
return diag.FromErr(err)
}
return nil
}

func resourceTstTDelete(ctx context.Context, d *schema.ResourceData, _ interface{}) diag.Diagnostics {
d.SetId("")
return nil
}