Fix tests on Windows

- Use echo hello world consistently. - Use cmd.exe /c echo hello world on Windows. - Stop using os.Clearenv() and instead selectively clear environment variables. Does not fix "shac check" on Windows yet. It will be done in a follow up. Change-Id: I366f42dabf6bc5899ba98c7a38c0afea519a28b2 Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/931712 Reviewed-by: Oliver Newman <[email protected]> Reviewed-by: Anthony Fandrianto <[email protected]> Commit-Queue: Oliver Newman <[email protected]>
shac-project · Oct 16, 2023 · 4a57f93 · 4a57f93
1 parent 308e502
commit 4a57f93
Show file tree

Hide file tree

Showing 8 changed files with 59 additions and 36 deletions.
diff --git a/internal/cli/main_test.go b/internal/cli/main_test.go
@@ -74,8 +74,12 @@ func init() {
 	helpOut = panicWrite{}
 	// Clear all environment variables to prevent automatic reporting mode
 	// selection, which can lead to inconsistent behavior depending on the
-	// environment.
-	os.Clearenv()
+	// environment. We cannot use os.Clearenv() since it breaks testing on
+	// Windows because TEMP is necessary for tests to succeed.
+	os.Unsetenv("GITHUB_RUN_ID")
+	os.Unsetenv("LUCI_CONTEXT")
+	os.Unsetenv("TERM")
+	os.Unsetenv("VSCODE_GIT_IPC_HANDLE")
 }
 
 func TestMainErr(t *testing.T) {

diff --git a/internal/engine/run_test.go b/internal/engine/run_test.go
@@ -826,9 +826,13 @@ func TestRun_Exec_InvalidPATHElements(t *testing.T) {
 	// launching succeeds. If shac doesn't handle invalid PATH elements
 	// correctly, launching any subprocess should fail, at least on platforms
 	// where filesystem sandboxing is supported.
+	cmd := "true"
+	if runtime.GOOS == "windows" {
+		cmd = "rundll32.exe"
+	}
 	writeFile(t, root, "shac.star", ""+
 		"def cb(ctx):\n"+
-		"    ctx.os.exec([\"true\"]).wait()\n"+
+		"    ctx.os.exec([\""+cmd+"\"]).wait()\n"+
 		"    print(\"success!\")\n"+
 		"shac.register_check(cb)\n")
 
@@ -1473,11 +1477,6 @@ func TestRun_SCM_Git_Recursive_Shared(t *testing.T) {
 // These test cases call fail() or throw an exception.
 func TestTestDataFailOrThrow(t *testing.T) {
 	t.Parallel()
-	// When running on Windows, the git installation may not have added the git
-	// environment. This is the case on M-A's personal workstation. In this case,
-	// some tests fail. Defaults to true since this is the case on GitHub Actions
-	// Windows worker.
-	isBashAvail := true
 	root, got := enumDir(t, "fail_or_throw")
 	data := []struct {
 		name  string
@@ -1694,17 +1693,17 @@ func TestTestDataFailOrThrow(t *testing.T) {
 		{
 			"ctx-os-exec-double_wait.star",
 			"wait: wait was already called",
-			"  //ctx-os-exec-double_wait.star:18:14: in cb\n",
+			"  //ctx-os-exec-double_wait.star:21:14: in cb\n",
 		},
 		{
 			"ctx-os-exec-false.star",
 			func() string {
-				if !isBashAvail && runtime.GOOS == "windows" {
-					return "ctx.os.exec: exec: \"false\": executable file not found in %PATH%"
+				if runtime.GOOS == "windows" {
+					return "wait: command failed with exit code 1: [cmd.exe /c exit 1]"
 				}
 				return "wait: command failed with exit code 1: [false]"
 			}(),
-			"  //ctx-os-exec-false.star:16:32: in cb\n",
+			"  //ctx-os-exec-false.star:19:26: in cb\n",
 		},
 		{
 			"ctx-os-exec-invalid_cwd.star",
@@ -1713,18 +1712,8 @@ func TestTestDataFailOrThrow(t *testing.T) {
 		},
 		{
 			"ctx-os-exec-mutate_result.star",
-			func() string {
-				if !isBashAvail && runtime.GOOS == "windows" {
-					return "ctx.os.exec: exec: \"echo\": executable file not found in %PATH%"
-				}
-				return "can't assign to .retcode field of struct"
-			}(),
-			func() string {
-				if !isBashAvail && runtime.GOOS == "windows" {
-					return "  //ctx-os-exec-mutate_result.star:16:22: in cb\n"
-				}
-				return "  //ctx-os-exec-mutate_result.star:17:8: in cb\n"
-			}(),
+			"can't assign to .retcode field of struct",
+			"  //ctx-os-exec-mutate_result.star:20:8: in cb\n",
 		},
 		{
 			"ctx-os-exec-no_cmd.star",
@@ -1733,7 +1722,13 @@ func TestTestDataFailOrThrow(t *testing.T) {
 		},
 		{
 			"ctx-os-exec-no_wait.star",
-			"wait() was not called on <subprocess \"echo hello world\">",
+			func() string {
+				cmd := "echo hello world"
+				if runtime.GOOS == "windows" {
+					cmd = "cmd.exe /c " + cmd
+				}
+				return "wait() was not called on <subprocess \"" + cmd + "\">"
+			}(),
 			"",
 		},
 		{
@@ -1759,7 +1754,7 @@ func TestTestDataFailOrThrow(t *testing.T) {
 		{
 			"ctx-os-exec-result_unhashable.star",
 			"unhashable type: subprocess",
-			"  //ctx-os-exec-result_unhashable.star:17:16: in cb\n",
+			"  //ctx-os-exec-result_unhashable.star:20:16: in cb\n",
 		},
 		{
 			"ctx-re-allmatches-no_arg.star",
@@ -2379,10 +2374,16 @@ func TestTestDataPrint(t *testing.T) {
 		},
 		{
 			name: "subprocess.star",
-			want: "[//subprocess.star:17] str(proc): <subprocess \"echo hello\">\n" +
-				"[//subprocess.star:18] type(proc): subprocess\n" +
-				"[//subprocess.star:19] bool(proc): True\n" +
-				"[//subprocess.star:20] dir(proc): [\"wait\"]\n",
+			want: func() string {
+				cmd := "echo hello world"
+				if runtime.GOOS == "windows" {
+					cmd = "cmd.exe /c " + cmd
+				}
+				return "[//subprocess.star:20] str(proc): <subprocess \"" + cmd + "\">\n" +
+					"[//subprocess.star:21] type(proc): subprocess\n" +
+					"[//subprocess.star:22] bool(proc): True\n" +
+					"[//subprocess.star:23] dir(proc): [\"wait\"]\n"
+			}(),
 		},
 		{
 			name: "true.star",

diff --git a/internal/engine/testdata/fail_or_throw/ctx-os-exec-double_wait.star b/internal/engine/testdata/fail_or_throw/ctx-os-exec-double_wait.star
@@ -13,7 +13,10 @@
 # limitations under the License.
 
 def cb(ctx):
-    proc = ctx.os.exec(["echo", "hello world"])
+    cmd = ["echo", "hello world"]
+    if ctx.platform.os == "windows":
+        cmd = ["cmd.exe", "/c"] + cmd
+    proc = ctx.os.exec(cmd)
     proc.wait()
     proc.wait()
 

diff --git a/internal/engine/testdata/fail_or_throw/ctx-os-exec-false.star b/internal/engine/testdata/fail_or_throw/ctx-os-exec-false.star
@@ -13,6 +13,9 @@
 # limitations under the License.
 
 def cb(ctx):
-    ctx.os.exec(["false"]).wait()
+    cmd = ["false"]
+    if ctx.platform.os == "windows":
+        cmd = ["cmd.exe", "/c", "exit", "1"]
+    ctx.os.exec(cmd).wait()
 
 shac.register_check(cb)
diff --git a/internal/engine/testdata/fail_or_throw/ctx-os-exec-mutate_result.star b/internal/engine/testdata/fail_or_throw/ctx-os-exec-mutate_result.star
@@ -13,7 +13,10 @@
 # limitations under the License.
 
 def cb(ctx):
-    res = ctx.os.exec(["echo", "hello world"]).wait()
+    cmd = ["echo", "hello world"]
+    if ctx.platform.os == "windows":
+        cmd = ["cmd.exe", "/c"] + cmd
+    res = ctx.os.exec(cmd).wait()
     res.retcode = 1
 
 shac.register_check(cb)
diff --git a/internal/engine/testdata/fail_or_throw/ctx-os-exec-no_wait.star b/internal/engine/testdata/fail_or_throw/ctx-os-exec-no_wait.star
@@ -13,6 +13,9 @@
 # limitations under the License.
 
 def cb(ctx):
-    ctx.os.exec(["echo", "hello world"])
+    cmd = ["echo", "hello world"]
+    if ctx.platform.os == "windows":
+        cmd = ["cmd.exe", "/c"] + cmd
+    ctx.os.exec(cmd)
 
 shac.register_check(cb)
diff --git a/internal/engine/testdata/fail_or_throw/ctx-os-exec-result_unhashable.star b/internal/engine/testdata/fail_or_throw/ctx-os-exec-result_unhashable.star
@@ -13,7 +13,10 @@
 # limitations under the License.
 
 def cb(ctx):
-    proc = ctx.os.exec(["echo", "hello world"])
+    cmd = ["echo", "hello world"]
+    if ctx.platform.os == "windows":
+        cmd = ["cmd.exe", "/c"] + cmd
+    proc = ctx.os.exec(cmd)
     print({proc: "should not be hashable"})
 
 shac.register_check(cb)
diff --git a/internal/engine/testdata/print/subprocess.star b/internal/engine/testdata/print/subprocess.star
@@ -13,7 +13,10 @@
 # limitations under the License.
 
 def cb(ctx):
-    proc = ctx.os.exec(["echo", "hello"])
+    cmd = ["echo", "hello world"]
+    if ctx.platform.os == "windows":
+        cmd = ["cmd.exe", "/c"] + cmd
+    proc = ctx.os.exec(cmd)
     print("str(proc): %s" % str(proc))
     print("type(proc): %s" % type(proc))
     print("bool(proc): %s" % bool(proc))