* Added ToolReturnCode to be more rigerous about how a return code is passed back from a tool (#911)

* Added support for a tool being able to pass back an 'ignored' result.
* Used enum codes to indicate meanings
* Made spawnAndWait return a ToolReturnCode

6 files changed, 112 insertions, 54 deletions

diff --git a/slang.h b/slang.h
index 5d9896073..7dc9501f0 100644
--- a/slang.h
+++ b/slang.h
@@ -668,6 +668,8 @@ extern "C"
 #define SLANG_E_NOT_FOUND                   SLANG_MAKE_CORE_ERROR(5)
     //! An unhandled internal failure (typically from unhandled exception)
 #define SLANG_E_INTERNAL_FAIL               SLANG_MAKE_CORE_ERROR(6)
+    //! Could not complete because some underlying feature (hardware or software) was not available 
+#define SLANG_E_NOT_AVAILABLE               SLANG_MAKE_CORE_ERROR(7)
 
     /** A "Universally Unique Identifier" (UUID)
 
diff --git a/source/core/slang-test-tool-util.cpp b/source/core/slang-test-tool-util.cpp
index 7ecfacefa..20ba2fc47 100644
--- a/source/core/slang-test-tool-util.cpp
+++ b/source/core/slang-test-tool-util.cpp
@@ -4,18 +4,34 @@
 namespace Slang
 {
 
-/* static */int TestToolUtil::getReturnCode(SlangResult res)
+/* static */ToolReturnCode TestToolUtil::getReturnCode(SlangResult res)
 {
-    if (SLANG_SUCCEEDED(res))
+    switch (res)
     {
-        return 0;
+        case SLANG_OK:              return ToolReturnCode::Success;
+        case SLANG_E_INTERNAL_FAIL: return ToolReturnCode::CompilationFailed;
+        case SLANG_FAIL:            return ToolReturnCode::Failed;
+        case SLANG_E_NOT_AVAILABLE: return ToolReturnCode::Ignored;
+        default:
+        {
+            return (SLANG_SUCCEEDED(res)) ? ToolReturnCode::Success : ToolReturnCode::Failed;
+        }
     }
-    else if (res == SLANG_E_INTERNAL_FAIL)
+}
+
+/* static */ToolReturnCode TestToolUtil::getReturnCodeFromInt(int code)
+{
+    if (code >= int(ToolReturnCodeSpan::First) && code <= int(ToolReturnCodeSpan::Last))
     {
-        return -1;
+        return ToolReturnCode(code);
+    }
+    else
+    {
+        SLANG_ASSERT(!"Invalid integral code");
+        return ToolReturnCode::Failed;
     }
-    return 1;
 }
 
+
 }
 
diff --git a/source/core/slang-test-tool-util.h b/source/core/slang-test-tool-util.h
index 615bbe10d..3ec655cad 100644
--- a/source/core/slang-test-tool-util.h
+++ b/source/core/slang-test-tool-util.h
@@ -11,13 +11,41 @@ namespace Slang {
 #   define SLANG_TEST_TOOL_API
 #endif
 
+/* When a tool is run as an executable the return code is the code returned from
+the last return of main. On unix this can be up to 8 bits. 
+By normal command line tool conventions returning 0 means success. */
+enum class ToolReturnCode
+{
+    CompilationFailed = -1,     ///< Compilation failure (-1 to maintain compatibility). This may still produce output and may mean a test was successful.
+    Success = 0,                ///< Tool ran normally
+    Failed,                     ///< Tool failed 
+    Ignored,                    ///< The run was ignored because it couldn't be run (because some optional feature was not present for example)
+    FailedToRun,                ///< Could not even run the test
+};
+
+enum class ToolReturnCodeSpan
+{
+    // Span of all valid values
+    First = int(ToolReturnCode::CompilationFailed),
+    Last = int(ToolReturnCode::FailedToRun),
+    // Span of all values that indicate the test is 'done'
+    FirstIsDone = int(ToolReturnCode::Ignored),
+    LastIsDone = int(ToolReturnCode::FailedToRun)
+};
+
 /* Utility functions for 'test tools' */
 struct TestToolUtil
 {
     typedef SlangResult(*InnerMainFunc)(Slang::StdWriters* stdWriters, SlangSession* session, int argc, const char*const* argv);
 
+        /// If the test failed to run or was ignored then we are done
+    static bool isDone(ToolReturnCode code) { return int(code) >= int(ToolReturnCodeSpan::FirstIsDone) && int(code) <= int(ToolReturnCodeSpan::LastIsDone); }
+
+        /// Convert from an int
+    static ToolReturnCode getReturnCodeFromInt(int code);
+
         /// Given a slang result, returns a return code that can be returned from an executable
-    static int getReturnCode(SlangResult res);
+    static ToolReturnCode getReturnCode(SlangResult res);
 };
 
 } // namespace Slang
diff --git a/source/slangc/main.cpp b/source/slangc/main.cpp
index bcc40b5ca..80b9cddbd 100644
--- a/source/slangc/main.cpp
+++ b/source/slangc/main.cpp
@@ -87,7 +87,7 @@ int MAIN(int argc, char** argv)
         res = innerMain(stdWriters, session, argc, argv);
         spDestroySession(session);
     }
-    return TestToolUtil::getReturnCode(res);
+    return (int)TestToolUtil::getReturnCode(res);
 }
 
 #ifdef _WIN32
diff --git a/tools/render-test/render-test-main.cpp b/tools/render-test/render-test-main.cpp
index 933641053..0be84f52f 100644
--- a/tools/render-test/render-test-main.cpp
+++ b/tools/render-test/render-test-main.cpp
@@ -574,7 +574,7 @@ SLANG_TEST_TOOL_API SlangResult innerMain(Slang::StdWriters* stdWriters, SlangSe
         }
     }
 
-    // Use the profilename set on options if set
+    // Use the profile name set on options if set
     profileName = gOptions.profileName ? gOptions.profileName : profileName;
 
     ShaderCompiler shaderCompiler;
@@ -670,7 +670,6 @@ SLANG_TEST_TOOL_API SlangResult innerMain(Slang::StdWriters* stdWriters, SlangSe
 	return SLANG_OK;
 }
 
-
 int main(int argc, char**  argv)
 {
     using namespace Slang;
@@ -681,6 +680,6 @@ int main(int argc, char**  argv)
     SlangResult res = innerMain(stdWriters, session, argc, argv);
     spDestroySession(session);
 
-	return SLANG_FAILED(res) ? 1 : 0;
+	return (int)TestToolUtil::getReturnCode(res);
 }
 
diff --git a/tools/slang-test/slang-test-main.cpp b/tools/slang-test/slang-test-main.cpp
index 285f77a72..d385e868d 100644
--- a/tools/slang-test/slang-test-main.cpp
+++ b/tools/slang-test/slang-test-main.cpp
@@ -437,7 +437,7 @@ OSError spawnAndWaitSharedLibrary(TestContext* context, const String& testPath,
         spawner.standardError_ = stdErrorString;
         spawner.standardOutput_ = stdOutString;
 
-        spawner.resultCode_ = TestToolUtil::getReturnCode(res);
+        spawner.resultCode_ = (int)TestToolUtil::getReturnCode(res);
 
         return kOSError_None;
     }
@@ -445,21 +445,35 @@ OSError spawnAndWaitSharedLibrary(TestContext* context, const String& testPath,
     return kOSError_OperationFailed;
 }
 
+ToolReturnCode getReturnCode(OSProcessSpawner& spawner)
+{
+    return TestToolUtil::getReturnCodeFromInt(spawner.getResultCode());
+}
 
-OSError spawnAndWait(TestContext* context, const String& testPath, SpawnType spawnType, OSProcessSpawner& spawner)
+ToolReturnCode spawnAndWait(TestContext* context, const String& testPath, SpawnType spawnType, OSProcessSpawner& spawner)
 {
+    OSError spawnResult = kOSError_OperationFailed;
     switch (spawnType)
     {
         case SpawnType::UseExe:
         {
-            return spawnAndWaitExe(context, testPath, spawner);
+            spawnResult = spawnAndWaitExe(context, testPath, spawner);
+            break;
         }
         case SpawnType::UseSharedLibrary:
         {
-            return spawnAndWaitSharedLibrary(context, testPath, spawner);
+            spawnResult = spawnAndWaitSharedLibrary(context, testPath, spawner);
+            break;
         }
+        default: break;
     }
-    return kOSError_OperationFailed;
+
+    if (spawnResult != kOSError_None)
+    {
+        return ToolReturnCode::FailedToRun;
+    }
+
+    return getReturnCode(spawner);
 }
 
 String getOutput(OSProcessSpawner& spawner)
@@ -530,6 +544,25 @@ static void _initSlangCompiler(TestContext* context, OSProcessSpawner& spawnerOu
     }
 }
 
+TestResult asTestResult(ToolReturnCode code)
+{
+    switch (code)
+    {
+        case ToolReturnCode::Success:               return TestResult::Pass;
+        case ToolReturnCode::Ignored:               return TestResult::Ignored;
+        default:                                    return TestResult::Fail;
+    }
+}
+
+#define TEST_RETURN_ON_DONE(x) \
+    { \
+        const ToolReturnCode toolRet_ = x; \
+        if (TestToolUtil::isDone(toolRet_)) \
+        { \
+            return asTestResult(toolRet_); \
+        } \
+    }
+
 TestResult runSimpleTest(TestContext* context, TestInput& input)
 {
     // need to execute the stand-alone Slang compiler on the file, and compare its output to what we expect
@@ -547,10 +580,7 @@ TestResult runSimpleTest(TestContext* context, TestInput& input)
         spawner.pushArgument(arg);
     }
 
-    if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     String actualOutput = getOutput(spawner);
 
@@ -621,10 +651,7 @@ TestResult runReflectionTest(TestContext* context, TestInput& input)
         spawner.pushArgument(arg);
     }
 
-    if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     String actualOutput = getOutput(spawner);
 
@@ -772,11 +799,8 @@ TestResult runCrossCompilerTest(TestContext* context, TestInput& input)
         expectedSpawner.pushArgument(arg);
     }
 
-    if (spawnAndWait(context, outputStem, input.spawnType, expectedSpawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
-
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, expectedSpawner));
+    
     String expectedOutput = getOutput(expectedSpawner);
     String expectedOutputPath = outputStem + ".expected";
     try
@@ -788,10 +812,8 @@ TestResult runCrossCompilerTest(TestContext* context, TestInput& input)
         return TestResult::Fail;
     }
 
-    if (spawnAndWait(context, outputStem, input.spawnType, actualSpawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, actualSpawner));
+
     String actualOutput = getOutput(actualSpawner);
 
     TestResult result = TestResult::Pass;
@@ -847,10 +869,7 @@ TestResult generateHLSLBaseline(TestContext* context, TestInput& input)
     spawner.pushArgument("-pass-through");
     spawner.pushArgument("fxc");
 
-    if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     String expectedOutput = getOutput(spawner);
     String expectedOutputPath = outputStem + ".expected";
@@ -895,10 +914,7 @@ TestResult runHLSLComparisonTest(TestContext* context, TestInput& input)
     spawner.pushArgument("-target");
     spawner.pushArgument("dxbc-assembly");
 
-    if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     // We ignore output to stdout, and only worry about what the compiler
     // wrote to stderr.
@@ -1004,10 +1020,7 @@ TestResult doGLSLComparisonTestRun(TestContext* context,
         spawner.pushArgument(arg);
     }
 
-    if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     OSProcessSpawner::ResultCode resultCode = spawner.getResultCode();
 
@@ -1043,6 +1056,13 @@ TestResult runGLSLComparisonTest(TestContext* context, TestInput& input)
     TestResult hlslResult   =  doGLSLComparisonTestRun(context, input, "__GLSL__",  "glslang", ".expected",    &expectedOutput);
     TestResult slangResult  =  doGLSLComparisonTestRun(context, input, "__SLANG__", nullptr,   ".actual",      &actualOutput);
 
+    // If either is ignored, the whole test is
+    if (hlslResult == TestResult::Ignored ||
+        slangResult == TestResult::Ignored)
+    {
+        return TestResult::Ignored;
+    }
+
     Slang::File::WriteAllText(outputStem + ".expected", expectedOutput);
     Slang::File::WriteAllText(outputStem + ".actual",   actualOutput);
 
@@ -1093,11 +1113,7 @@ TestResult runComputeComparisonImpl(TestContext* context, TestInput& input, cons
     // clear the stale actual output file first. This will allow us to detect error if render-test fails and outputs nothing.
     File::WriteAllText(actualOutputFile, "");
 
-	if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-	{
-        printf("error spawning render-test\n");
-		return TestResult::Fail;
-	}
+	TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     auto actualOutput = getOutput(spawner);
     auto expectedOutput = getExpectedOutput(outputStem);
@@ -1192,10 +1208,7 @@ TestResult doRenderComparisonTestRun(TestContext* context, TestInput& input, cha
     spawner.pushArgument("-o");
     spawner.pushArgument(outputStem + outputKind + ".png");
 
-    if (spawnAndWait(context, outputStem, input.spawnType, spawner) != kOSError_None)
-    {
-        return TestResult::Fail;
-    }
+    TEST_RETURN_ON_DONE(spawnAndWait(context, outputStem, input.spawnType, spawner));
 
     OSProcessSpawner::ResultCode resultCode = spawner.getResultCode();