Runtime: Improve the failure mode for CLI commands

Previously, the CLI always exited with EXIT_SUCCESS even if the command displayed the usage info (a failure mode that should probably result in EXIT_FAILURE). This is unfortunate because it means other programs can't detect that a command, e.g. a build or test runner, has failed. The snapshot test suite has been updated to receive more inputs, but the `tmpfile` thing is a bit sketchy.

Runtime/API/C_Runtime.lua

@@ -1,5 +1,6 @@
 local assertions = require("assertions")
 local bdd = require("bdd")
+local ffi = require("ffi")
 local transform = require("transform")
 local validation = require("validation")
 
@@ -75,14 +76,24 @@ function C_Runtime.PrintVersionString()
 	print(EVO_VERSION)
 end
 
-local function shell_exec(shellCommand)
-	local file = assert(io.popen(shellCommand, "r"))
+local function shell_exec(command)
+	local tempFile = os.tmpname()
+	local commandWithRedirection = command .. " > " .. tempFile .. " 2>&1"
 
-	file:flush() -- Required to prevent receiving only partial output
+	-- Somewhat sketchy, but portable enough for this use case?
+	local success, terminationReason, exitCode = os.execute(commandWithRedirection)
+
+	local file = io.open(tempFile, "rb")
 	local output = file:read("*all")
 	file:close()
 
-	return output
+	os.remove(tempFile)
+
+	if ffi.os == "Windows" then
+		-- Have to normalize to get rid of MS idiosyncracies
+		output = output:gsub("%s?\r\n", "\n")
+	end
+	return output, success, terminationReason, exitCode
 end
 
 function C_Runtime.RunSnapshotTests(testCases)
@@ -94,9 +105,12 @@ function C_Runtime.RunSnapshotTests(testCases)
 
 		printf("Running snapshot test %s", transform.bold(descriptor))
 		printf("[SHELL] Executing command: %s", transform.green(testInfo.programToRun))
-		local observedOutput = shell_exec(testInfo.programToRun)
-		testInfo.onExit(observedOutput)
+		local observedOutput, status, terminationReason, exitCodeOrSignalID = shell_exec(testInfo.programToRun)
 		printf("[SHELL] Observed output has %d bytes", #observedOutput)
+		printf("[SHELL] Termination status: %s", status)
+		printf("[SHELL] Termination reason: %s", terminationReason)
+		printf("[SHELL] Return code or signal: %s", exitCodeOrSignalID)
+		testInfo.onExit(observedOutput, status, terminationReason, exitCodeOrSignalID)
 	end
 end
 

Runtime/evo.lua

@@ -310,7 +310,7 @@ function evo.buildZipApp(commandName, argv)
 		printf(transform.brightRed(evo.errorStrings.APP_BUNDLER_INVALID_BUILD_DIR), outputFileName, inputDirectory)
 		print()
 		print(evo.messageStrings.BUILD_COMMAND_USAGE_INFO)
-		return
+		os.exit(EXIT_FAILURE)
 	end
 
 	if #argv == 0 then
@@ -329,7 +329,7 @@ function evo.buildZipApp(commandName, argv)
 		)
 		print()
 		print(evo.messageStrings.BUILD_COMMAND_USAGE_INFO)
-		return
+		os.exit(EXIT_FAILURE)
 	end
 
 	local zipWriter = miniz.new_writer()
@@ -409,7 +409,7 @@ function evo.discoverAndRunTests(command, argv)
 			print(transform.brightRed(evo.errorStrings.TEST_RUNNER_ENTRY_POINT_MISSING))
 			print()
 			print(evo.messageStrings.TEST_COMMAND_USAGE_INFO)
-			return
+			os.exit(EXIT_FAILURE)
 		end
 
 		_G.arg = appArgs

Tests/snapshot-test.lua

@@ -7,11 +7,27 @@ local transform = require("transform")
 local vfs = require("vfs")
 
 local assertEquals = assertions.assertEquals
+local assertNil = assertions.assertNil
 local assertTrue = assertions.assertTrue
 
 local isWindows = ffi.os == "Windows"
 local EXECUTABLE_SUFFIX = isWindows and ".exe" or ""
 
+local EXIT_SUCCESS = 0
+local EXIT_FAILURE = 1
+
+local function assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
+	assertTrue(status)
+	assertEquals(terminationReason, "exit")
+	assertEquals(exitCodeOrSignalID, EXIT_SUCCESS)
+end
+
+local function assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
+	assertNil(status)
+	assertEquals(terminationReason, "exit")
+	assertEquals(exitCodeOrSignalID, EXIT_FAILURE)
+end
+
 local function fixUpReportString(reportString)
 	-- Relying on timings to be identical will only cause flakiness
 	return reportString:gsub("complete %(%d* ms%)", "complete (0 ms)")
@@ -36,61 +52,67 @@ local testCases = {
 	["cli-no-args"] = {
 		humanReadableDescription = "Invoking the CLI without passing any arguments should print the help text",
 		programToRun = "evo",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local helpText = evo.getHelpText()
 			local versionText = evo.getVersionText()
 			local documentationLinkText = evo.messageStrings.HELP_COMMAND_DOCUMENTATION_LINK
 
 			assertEquals(observedOutput, helpText .. "\n" .. versionText .. "\n" .. documentationLinkText .. "\n")
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-run-script"] = {
 		humanReadableDescription = "Invoking the CLI with a Lua script path should execute the script",
 		programToRun = "evo Tests/Fixtures/hello-world-app/main.lua",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = "Hello world!\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-help-text"] = {
 		humanReadableDescription = "Invoking the CLI help command should print the help text",
 		programToRun = "evo help",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local helpText = evo.getHelpText()
 			local versionText = evo.getVersionText()
 			local documentationLinkText = evo.messageStrings.HELP_COMMAND_DOCUMENTATION_LINK
 
 			assertEquals(observedOutput, helpText .. "\n" .. versionText .. "\n" .. documentationLinkText .. "\n")
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-version-text"] = {
 		humanReadableDescription = "Invoking the CLI version command should print the runtime version only",
 		programToRun = "evo version",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local VERSION_PATTERN = "(%d+.%d+.%d+).*"
 			local runtimeVersion = observedOutput:match("^v" .. VERSION_PATTERN .. "$")
 			local hasRuntimeVersion = (runtimeVersion ~= nil)
 			assertTrue(hasRuntimeVersion)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-eval-nil"] = {
 		humanReadableDescription = "Invoking the CLI eval command with no arguments should print nothing",
 		programToRun = "evo eval",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			assertEquals(observedOutput, "")
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-eval-chunk"] = {
 		humanReadableDescription = "Invoking the CLI eval command with a valid chunk should print the result",
 		programToRun = 'evo eval "print(42)"',
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			assertEquals(observedOutput, "42\n")
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-build-cwd"] = {
 		humanReadableDescription = "Invoking the CLI build command with no arguments should try to build from cwd",
 		programToRun = "evo build",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local executableName = "evo-runtime" .. EXECUTABLE_SUFFIX
 			local expectedOutput = format("Building from %s", transform.bold(uv.cwd()))
 				.. "\n"
@@ -103,12 +125,13 @@ local testCases = {
 				.. evo.messageStrings.BUILD_COMMAND_USAGE_INFO
 				.. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-build-success"] = {
 		humanReadableDescription = "Invoking the CLI build command with a valid app directory should build an executable",
 		programToRun = "evo build Tests/Fixtures/hello-world-app",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local executableName = "hello-world-app" .. EXECUTABLE_SUFFIX
 			local fullAppDirectoryPath = path.join(uv.cwd(), "Tests/Fixtures/hello-world-app")
 
@@ -142,25 +165,27 @@ local testCases = {
 				.. "\n"
 				.. format("Created self-contained executable: %s\n", transform.brightYellow(executableName))
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-build-invalid-dir"] = {
 		humanReadableDescription = "Invoking the CLI build command while passing an invalid directory should fail",
 		programToRun = "evo build does-not-exist",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local executableName = "does-not-exist" .. EXECUTABLE_SUFFIX
 			local expectedOutput = format(
 				transform.brightRed(evo.errorStrings.APP_BUNDLER_INVALID_BUILD_DIR),
 				executableName,
 				"does-not-exist"
 			) .. "\n\n" .. evo.messageStrings.BUILD_COMMAND_USAGE_INFO .. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-build-file"] = {
 		humanReadableDescription = "Invoking the CLI build command while passing a file instead of a directory should fail",
 		programToRun = "evo build Tests/Fixtures/hello-world-app/main.lua",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local executableName = "main.lua" .. EXECUTABLE_SUFFIX
 			local inputFilePath = "Tests/Fixtures/hello-world-app/main.lua"
 			local expectedOutput = format(
@@ -169,34 +194,37 @@ local testCases = {
 				inputFilePath
 			) .. "\n\n" .. evo.messageStrings.BUILD_COMMAND_USAGE_INFO .. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-noargs-error"] = {
 		humanReadableDescription = "Invoking the test command without args should print an error if test.lua doesn't exist",
 		programToRun = "evo test",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = transform.brightRed(evo.errorStrings.TEST_RUNNER_ENTRY_POINT_MISSING)
 				.. "\n\n"
 				.. evo.messageStrings.TEST_COMMAND_USAGE_INFO
 				.. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-noargs-error-with-app-args"] = {
 		humanReadableDescription = "Invoking the test command without args but with app args should print an error if test.lua doesn't exist",
 		programToRun = "evo test --integration",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = transform.brightRed(evo.errorStrings.TEST_RUNNER_ENTRY_POINT_MISSING)
 				.. "\n\n"
 				.. evo.messageStrings.TEST_COMMAND_USAGE_INFO
 				.. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-multiple-existing-files"] = {
 		humanReadableDescription = "Invoking the test command with existing file paths should load them as tests",
 		programToRun = "evo test Tests/Fixtures/test-dir/subdir/lua-test-file.lua Tests/Fixtures/test-dir/lua-spec-file.spec.lua",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local specFiles = {
 				"Tests/Fixtures/test-dir/subdir/lua-test-file.lua",
 				"Tests/Fixtures/test-dir/lua-spec-file.spec.lua",
@@ -212,12 +240,13 @@ local testCases = {
 
 			observedOutput = fixUpReportString(observedOutput)
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-existing-files-and-dirs"] = {
 		humanReadableDescription = "Invoking the test command with existing file and directory paths should load all included .lua files as tests",
 		programToRun = "evo test Tests/Fixtures/test-dir/lua-spec-file.spec.lua Tests/Fixtures/test-dir",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local specFiles = {
 				"Tests/Fixtures/test-dir/lua-spec-file.spec.lua",
 				"Tests/Fixtures/test-dir/lua-spec-file.spec.lua",
@@ -237,37 +266,41 @@ local testCases = {
 
 			observedOutput = fixUpReportString(observedOutput)
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 
 	["cli-test-cannot-load-file"] = {
 		humanReadableDescription = "Invoking the test command with non-Lua script files should fail with an error",
 		programToRun = "evo test Tests/Fixtures/test-dir/lua-spec-file.spec.lua Tests/Fixtures/test-dir/not-a-lua-file.txt",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = transform.brightRed(
 				format(evo.errorStrings.TEST_RUNNER_CANNOT_LOAD, "Tests/Fixtures/test-dir/not-a-lua-file.txt")
 			) .. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-cannot-open-file"] = {
 		humanReadableDescription = "Invoking the test command with invalid file paths should fail with an error",
 		programToRun = "evo test Tests/Fixtures/test-dir/lua-spec-file.spec.lua Tests/Fixtures/test-dir/does-not-exist.lua",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = transform.brightRed(
 				format(evo.errorStrings.TEST_RUNNER_CANNOT_OPEN, "Tests/Fixtures/test-dir/does-not-exist.lua")
 			) .. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-cannot-open-dir"] = {
 		humanReadableDescription = "Invoking the test command with invalid directory paths should fail with an error",
 		programToRun = "evo test Tests/Fixtures/test-dir/lua-spec-file.spec.lua Tests/Fixtures/test-dir/does-not-exist",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = transform.brightRed(
 				format(evo.errorStrings.TEST_RUNNER_CANNOT_OPEN, "Tests/Fixtures/test-dir/does-not-exist")
 			) .. "\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitFailure(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 }
@@ -279,34 +312,38 @@ testCases = {
 	["cli-hello-world-app"] = {
 		humanReadableDescription = "Invoking the hello world app should execute the bundled app instead of the runtime CLI",
 		programToRun = ffi.os ~= "Windows" and "chmod +x hello-world-app && ./hello-world-app" or "hello-world-app.exe",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			assertEquals(observedOutput, "Hello world!\n")
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	-- This one interferes with the build tests as it requires creating temporary files
 	["cli-run-cwd"] = {
 		humanReadableDescription = "Invoking the CLI with a single dot should execute main.lua",
 		programToRun = "evo .",
 		-- Assumes the entry point has been created prior to running the test suite, which it should have
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = "Hello from main.lua\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-noargs"] = {
 		humanReadableDescription = "Invoking the test command without args should run test.lua if it exists",
 		programToRun = "evo test",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = "Hello from test.lua (app args: 0, nil)\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 	["cli-test-noargs-forward"] = {
 		humanReadableDescription = "Invoking the test command without args but with app args should run test.lua and forward the app args",
 		programToRun = "evo test --integration",
-		onExit = function(observedOutput)
+		onExit = function(observedOutput, status, terminationReason, exitCodeOrSignalID)
 			local expectedOutput = "Hello from test.lua (app args: 1, integration)\n"
 			assertEquals(observedOutput, expectedOutput)
+			assertExitSuccess(observedOutput, status, terminationReason, exitCodeOrSignalID)
 		end,
 	},
 }