* Made 'sub command' git-like - with parameters before going to slang-test and after going to the tool (#764)

* Document some of the changes to command line invocation
* Make -v option display the effective command that is being used to run the test

4 files changed, 114 insertions, 91 deletions

diff --git a/tools/slang-test/README.md b/tools/slang-test/README.md
index 1dc44a8b8..9dd46b193 100644
--- a/tools/slang-test/README.md
+++ b/tools/slang-test/README.md
@@ -1,6 +1,6 @@
 # Slang Test
 
-Slang Test is a command line tool that is used to coordinate tests via other command line tools. The actual executable is 'slang-test'. It is typically run from the test.bat script in the root directory of the project.
+Slang Test is a command line tool that is used to coordinate tests via other tools. The actual executable is 'slang-test'. It is typically run from the test.bat script in the root directory of the project.
 
 Slang Test can be thought of as the 'hub' running multiple tests and accumulating the results. In the distribution tests are held in the 'tests' directory. Inside this directory there are tests grouped together via other directories. Inside those directories are the actual tests themselves. The tests exist as .hlsl, .slang and .glsl and other file extensions. The top line of each of these files describe what kind of test will be performed with a specialized comment '//TEST'. 
 
@@ -16,6 +16,20 @@ slang-test -bindir E:\slang\bin\windows-x64\Debug\\ -category full tests/compute
 * The -category full means that all tests can be run.
 * The final 'free parameter' is 'tests/compute/array-param' and means only tests starting with this string will run.
 
+Most types of test use 'test tools' to implement actual tests. There are currently 3 'tools' that are typically used 
+
+* slangc
+* render-test
+* slang-reflection-test
+
+These are typically implemeted as dlls/shared libraries that are loaded when a test is needed. Sometimes it is necessary or useful to just call one of these test tools directly with the parameters the tool takes. This can be achieved by giving the tool as a 'sub command' name on the command line. All of the parameters after the tool name will be passed directly to the tool. For example
+
+ ```
+slang-test -bindir E:\slang\bin\windows-x64\Debug\\ slangc tests/compute/array-param.slang
+```
+
+Will run the 'slangc' tool with the parameters listed after 'slangc' on the command line. Any parameters before the sub command will be parsed as usual by slang-test, and if not applicable to invoking the tool will be ignored. bindir will be used for finding the tool directory. This is by design so that the sub command invocation can just be placed after the normal slang-test commands, and removed when no longer needed. 
+
 The command line can control which tests are run with a couple of switches
 
 * -api - Overall controls over which apis will be tested against 
@@ -54,6 +68,7 @@ There are the following test categories
 * render
 * compute
 * vulkan
+* compatibility-issue
 
 A test may be in one or more categories. The categories are specified in the test line, for example: 
 //TEST(smoke,compute):COMPARE_COMPUTE:
diff --git a/tools/slang-test/main.cpp b/tools/slang-test/main.cpp
index a5578ada2..07647e58f 100644
--- a/tools/slang-test/main.cpp
+++ b/tools/slang-test/main.cpp
@@ -347,18 +347,35 @@ TestResult gatherTestsForFile(
 
 OSError spawnAndWait(TestContext* context, const String& testPath, OSProcessSpawner& spawner)
 {
-    SLANG_UNUSED(context);
-
-    if(context->options.shouldBeVerbose)
-    {
-        String commandLine = spawner.getCommandLine();
-        context->reporter->messageFormat(TestMessageType::Info, "%s\n", commandLine.begin());
-    }
-
-    if (!context->options.useExes)
+    const auto& options = context->options;
+    
+    if (!options.useExes)
     {
         String exeName = Path::GetFileNameWithoutEXT(spawner.executableName_);
 
+        if (options.shouldBeVerbose)
+        {
+            StringBuilder builder;
+
+            builder << "slang-test";
+
+            if (options.binDir)
+            {
+                builder << " -bindir " << options.binDir; 
+            }
+
+            builder << " " << exeName;
+
+            // TODO(js): Potentially this should handle escaping parameters for the command line if need be
+            const auto& argList = spawner.argumentList_;
+            for (UInt i = 0; i < argList.Count(); ++i)
+            {
+                builder << " " << argList[i];
+            }
+
+            context->reporter->messageFormat(TestMessageType::Info, "%s\n", builder.begin());
+        }
+
         auto func = context->getInnerMainFunc(String(context->options.binDir), exeName);
         if (func)
         {
@@ -396,6 +413,13 @@ OSError spawnAndWait(TestContext* context, const String& testPath, OSProcessSpaw
         }
     }
 
+    if (options.shouldBeVerbose)
+    {
+        String commandLine = spawner.getCommandLine();
+        context->reporter->messageFormat(TestMessageType::Info, "%s\n", commandLine.begin());
+    }
+
+
     OSError err = spawner.spawnAndWaitForCompletion();
     if (err != kOSError_None)
     {
@@ -1737,80 +1761,6 @@ void runTestsInDirectory(
     }
 }
 
-struct ToolInvoke
-{
-        /// Returns true if it is a tool invoke
-    bool parse(const char*const* argv, int argc)
-    {
-        m_args.Clear();
-
-        m_binDirectory = ".";
-
-        if (argc < 2 || !_isToolName(argv[1]))
-        {
-            return false;
-        }
-        m_toolName = argv[1];
-
-        // Look for parameters that are for the slang-test, and should be skipped
-        int i = 2;
-        while (i < argc)
-        {
-            if (strcmp(argv[i], "-bindir") == 0 && i + 1 < argc)
-            {
-                m_binDirectory = argv[i + 1];
-                i += 2;
-            }
-            // If nothing found, the rest must be parsed to the tool
-            break;
-        }
-
-        m_args.Add(m_toolName.Buffer());
-        m_args.AddRange(argv + i, argc - i);
-        return true;
-    }
-
-    SlangResult invoke(AppContext* appContext, TestContext* testContext)
-    {
-        // Do I want to strip the -bindir directory that may be later
-
-            // We will just parse everything onto the underlying tool
-        auto func = testContext->getInnerMainFunc(m_binDirectory, m_toolName);
-        if (!func)
-        {
-            AppContext::getStdError().print("error: Unable to launch tool '%s'\n", m_toolName.Buffer());
-            return SLANG_FAIL;
-        }
-
-        return func(AppContext::getSingleton(), testContext->getSession(), int(m_args.Count()), m_args.Buffer());
-    }
-
-    String m_binDirectory;
-    String m_toolName;
-    List<const char*> m_args;
-
-private:
-    static bool _isToolName(const char* name)
-    {
-        static const char* toolNames[] =
-        {
-            "slangc",
-            "render-test",
-            "slang-reflection-test",
-        };
-
-        for (int i = 0; i < SLANG_COUNT_OF(toolNames); ++i)
-        {
-            if (::strcmp(toolNames[i], name) == 0)
-            {
-                return true;
-            }
-        }
-        return false;
-    }
-};
-
-
 SlangResult innerMain(int argc, char** argv)
 {
     AppContext::initDefault();
@@ -1839,17 +1789,32 @@ SlangResult innerMain(int argc, char** argv)
         context.setInnerMainFunc("slangc", &SlangCTool::innerMain);
     }
 
+    SLANG_RETURN_ON_FAIL(Options::parse(argc, argv, &categorySet, AppContext::getStdError(), &context.options));
+    
+    Options& options = context.options;
+
+    if (options.subCommand.Length())
     {
-        ToolInvoke toolInvoke;
-        if (toolInvoke.parse(argv, argc))
+        // Get the function from the tool
+        auto func = context.getInnerMainFunc(options.binDir, options.subCommand);
+        if (!func)
+        {
+            AppContext::getStdError().print("error: Unable to launch tool '%s'\n", options.subCommand.Buffer());
+            return SLANG_FAIL;
+        }
+
+        // Copy args to a char* list
+        const auto& srcArgs = options.subCommandArgs;
+        List<const char*> args;
+        args.SetSize(srcArgs.Count());
+        for (UInt i = 0; i < srcArgs.Count(); ++i)
         {
-            return toolInvoke.invoke(AppContext::getSingleton(), &context);
+            args[i] = srcArgs[i].Buffer();
         }
+
+        return func(AppContext::getSingleton(), context.getSession(), int(args.Count()), args.Buffer());
     }
 
-    SLANG_RETURN_ON_FAIL(Options::parse(argc, argv, &categorySet, AppContext::getStdError(), &context.options));
-    
-    Options& options = context.options;
     if( options.includeCategories.Count() == 0 )
     {
         options.includeCategories.Add(fullTestCategory, fullTestCategory);
diff --git a/tools/slang-test/options.cpp b/tools/slang-test/options.cpp
index cb6adbc6f..6f3f5d79b 100644
--- a/tools/slang-test/options.cpp
+++ b/tools/slang-test/options.cpp
@@ -42,6 +42,23 @@ TestCategory* TestCategorySet::findOrError(String const& name)
     return category;
 }
 
+/* We need a way to differentiate a subCommand from say a test prefix. Here
+we assume a command is just alpha characters or -, and this would differentiate it from
+typical prefix usage (which is generally a directory). */
+static bool _isSubCommand(const char* arg)
+{
+    for (; *arg; arg++)
+    {
+        const char c = *arg;
+        // A command is just letters
+        if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '-'))
+        {
+            return false;
+        }
+    }
+    return true;
+}
+
 /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!! Options !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
 
 /* static */Result Options::parse(int argc, char** argv, TestCategorySet* categorySet, Slang::WriterHelper stdError, Options* optionsOut)
@@ -67,12 +84,32 @@ TestCategory* TestCategorySet::findOrError(String const& name)
         char const* arg = *argCursor++;
         if (arg[0] != '-')
         {
+            // We need to determine if this is a command, the confusion is that
+            // previously we can specify a test prefix as just a single positional arg.
+            // To rule this out, here it can only be a subCommand if it is just text
+
+            if (_isSubCommand(arg))
+            {
+                optionsOut->subCommand = arg;
+                // Make the first arg the command name
+                optionsOut->subCommandArgs.Add(optionsOut->subCommand);
+
+                // Add all the remaining commands to subCommands
+                for (; argCursor != argEnd; ++argCursor)
+                {
+                    optionsOut->subCommandArgs.Add(*argCursor);
+                }
+                // Done
+                return SLANG_OK;
+            }
+
             positionalArgs.Add(arg);
             continue;
         }
 
         if (strcmp(arg, "--") == 0)
         {
+            // Add all positional args at the end
             while (argCursor != argEnd)
             {
                 positionalArgs.Add(*argCursor++);
diff --git a/tools/slang-test/options.h b/tools/slang-test/options.h
index ccbd9aede..4858ffd03 100644
--- a/tools/slang-test/options.h
+++ b/tools/slang-test/options.h
@@ -43,7 +43,7 @@ struct Options
     // Directory to use when looking for binaries to run
     char const* binDir = "";
 
-    // only run test cases with names that have this prefix
+    // only run test cases with names that have this prefix. 
     char const* testPrefix = nullptr;
 
     // generate extra output (notably: command lines we run)
@@ -72,6 +72,12 @@ struct Options
     // By default we can test against all apis
     RenderApiFlags enabledApis = RenderApiFlag::AllOf;
 
+    // The subCommand to execute. Will be empty if there is no subCommand 
+    Slang::String subCommand;      
+
+    // Arguments to the sub command. Note that if there is a subCommand the first parameter is always the subCommand itself.
+    Slang::List<Slang::String> subCommandArgs;
+
     // By default we potentially synthesize test for all 
     // TODO: Vulkan is disabled by default for now as the majority as vulkan synthesized tests fail  
     RenderApiFlags synthesizedTestApis = RenderApiFlag::AllOf & ~RenderApiFlag::Vulkan;