From d1a5dfa1addacaace03cb14f2e2eff0a25485b43 Mon Sep 17 00:00:00 2001
From: kmoe <5575356+kmoe@users.noreply.github.com>
Date: Thu, 6 Jul 2023 19:28:09 +0100
Subject: [PATCH] command: test plan -refresh= arg ordering (#33483)

* main: disambiguate arg ordering test

Make it extra clear what order of args we are asserting.

* command: fix plan -refresh=false test

The test for plan -refresh=false was not functioning, since ReadResource will not be called if the resource is not in prior state.

Add a new fixture directory with state, and also test the converse, to prevent regression.

* command: add test for refresh flag precedence

A consumer relies on the fact that running terraform plan -refresh=false -refresh true gives the same result as terraform plan -refresh=true.
---
 internal/command/plan_test.go                 | 67 ++++++++++++++++++-
 .../testdata/plan-existing-state/main.tf      | 13 ++++
 .../plan-existing-state/terraform.tfstate     | 23 +++++++
 main_test.go                                  |  6 +-
 4 files changed, 105 insertions(+), 4 deletions(-)
 create mode 100644 internal/command/testdata/plan-existing-state/main.tf
 create mode 100644 internal/command/testdata/plan-existing-state/terraform.tfstate

diff --git a/internal/command/plan_test.go b/internal/command/plan_test.go
index 7d83070246..d7f25dd403 100644
--- a/internal/command/plan_test.go
+++ b/internal/command/plan_test.go
@@ -472,7 +472,7 @@ func TestPlan_outBackend(t *testing.T) {
 func TestPlan_refreshFalse(t *testing.T) {
 	// Create a temporary working directory that is empty
 	td := t.TempDir()
-	testCopyDir(t, testFixturePath("plan"), td)
+	testCopyDir(t, testFixturePath("plan-existing-state"), td)
 	defer testChdir(t, td)()
 
 	p := planFixtureProvider()
@@ -498,6 +498,71 @@ func TestPlan_refreshFalse(t *testing.T) {
 	}
 }
 
+func TestPlan_refreshTrue(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath("plan-existing-state"), td)
+	defer testChdir(t, td)()
+
+	p := planFixtureProvider()
+	view, done := testView(t)
+	c := &PlanCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			View:             view,
+		},
+	}
+
+	args := []string{
+		"-refresh=true",
+	}
+	code := c.Run(args)
+	output := done(t)
+	if code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, output.Stderr())
+	}
+
+	if !p.ReadResourceCalled {
+		t.Fatalf("ReadResource should have been called")
+	}
+}
+
+// A consumer relies on the fact that running
+// terraform plan -refresh=false -refresh=true gives the same result as
+// terraform plan -refresh=true.
+// While the flag logic itself is handled by the stdlib flags package (and code
+// in main() that is tested elsewhere), we verify the overall plan command
+// behaviour here in case we accidentally break this with additional logic.
+func TestPlan_refreshFalseRefreshTrue(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := t.TempDir()
+	testCopyDir(t, testFixturePath("plan-existing-state"), td)
+	defer testChdir(t, td)()
+
+	p := planFixtureProvider()
+	view, done := testView(t)
+	c := &PlanCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			View:             view,
+		},
+	}
+
+	args := []string{
+		"-refresh=false",
+		"-refresh=true",
+	}
+	code := c.Run(args)
+	output := done(t)
+	if code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, output.Stderr())
+	}
+
+	if !p.ReadResourceCalled {
+		t.Fatal("ReadResource should have been called")
+	}
+}
+
 func TestPlan_state(t *testing.T) {
 	// Create a temporary working directory that is empty
 	td := t.TempDir()
diff --git a/internal/command/testdata/plan-existing-state/main.tf b/internal/command/testdata/plan-existing-state/main.tf
new file mode 100644
index 0000000000..7b30915731
--- /dev/null
+++ b/internal/command/testdata/plan-existing-state/main.tf
@@ -0,0 +1,13 @@
+resource "test_instance" "foo" {
+  ami = "bar"
+
+  # This is here because at some point it caused a test failure
+  network_interface {
+    device_index = 0
+    description  = "Main network interface"
+  }
+}
+
+data "test_data_source" "a" {
+  id = "zzzzz"
+}
diff --git a/internal/command/testdata/plan-existing-state/terraform.tfstate b/internal/command/testdata/plan-existing-state/terraform.tfstate
new file mode 100644
index 0000000000..e81bd9b869
--- /dev/null
+++ b/internal/command/testdata/plan-existing-state/terraform.tfstate
@@ -0,0 +1,23 @@
+{
+  "version": 4,
+  "terraform_version": "1.6.0",
+  "serial": 1,
+  "lineage": "d496625c-bde2-aebc-f5f4-ebbf54eabed2",
+  "outputs": {},
+   "resources": [
+      {
+        "module": "module.child",
+        "mode": "managed",
+        "type": "test_instance",
+        "name": "test",
+        "provider": "provider[\"registry.terraform.io/hashicorp/test\"]",
+        "instances": [
+          {
+            "schema_version": 0,
+            "attributes": {}
+          }
+        ]
+      }
+    ],
+  "check_results": null
+}
diff --git a/main_test.go b/main_test.go
index e1eb64baa5..0a0834a67c 100644
--- a/main_test.go
+++ b/main_test.go
@@ -49,8 +49,8 @@ func TestMain_cliArgsFromEnv(t *testing.T) {
 		{
 			"both env var and CLI",
 			[]string{testCommandName, "foo", "bar"},
-			"-foo bar",
-			[]string{"-foo", "bar", "foo", "bar"},
+			"-foo baz",
+			[]string{"-foo", "baz", "foo", "bar"},
 			false,
 		},
 
@@ -143,7 +143,7 @@ func TestMain_cliArgsFromEnv(t *testing.T) {
 
 			// Verify
 			if !reflect.DeepEqual(testCommand.Args, tc.Expected) {
-				t.Fatalf("bad: %#v", testCommand.Args)
+				t.Fatalf("expected args %#v but got %#v", tc.Expected, testCommand.Args)
 			}
 		})
 	}