[lit, lldbsuite] Update the lldbsuite to correctly run tests on windows and windows server

[stella.stamenova]

Summary

The new script to run the lldbtests as part of lit invokes each test by calling dotest.py, however, we cannot rely on the system to always correctly interpret the script as python causing the tests to be unresolved on windows (at least). To fix this, we need to make sure that the first parameter in the command line is the python executable itself.

In Makefile.rules, there are a number of windows specific definitions that rely on the HOST_OS being set as Windows_NT but the logic detecting the OS currently does not detect server versions of windows correctly. This change updates the logic to detect windows server as well.

[JDevlieghere]

Thanks Stella, LGTM. Apologies for breaking all this Windows stuff! :(

[labath]

lit/Suite/lldbtest.py

47

Do we need to worry about picking the correct python/python_d for release vs. debug builds?

[zturner]

lit/Suite/lldbtest.py

47

If it's a debug build, sys.executable should already be python_d.exe. So this should be correct.

My question though, is where does self.dotest_cmd come from? Would it ever be the case that this already has the python executable? Then you'd have an error because it would be specified twice.

[stella.stamenova]

lit/Suite/lldbtest.py

47

Currently, dotest_cmd is hardcoded in the lldb suite lit.cfg to point to dotest_path (+some arguments) and dotest_path itself is hardcoded in lldb-dotest.in to point to the dotest.py script.

I considered testing whether the command contained python before adding it, but since the command is currently hardcoded to point to dotest.py, it seemed wasteful.

[zturner]

lit/Suite/lldbtest.py

47

This seems fine to me then. I have a mild preference for prepending sys.executable from inside of lit.cfg and then putting an assertion here that sys.executable is present as the first argument, but it's up to you, if you like that too you can do it, otherwise this is fine as is IMO.

[labath]

lit/Suite/lldbtest.py

47

If it's a debug build, sys.executable should already be python_d.exe. So this should be correct.

I was worried that the python executable we use to run lit may not be the same one as we need to use for lldb testing in a specific build configuration (I remember there being some confusion about that in the past). But If you think it fine, then I'm ok as well. (it only matters for VS builds anyway).

lit/Suite/lldbtest.py

class LLDBTest(TestFormat):
     def execute(self, test, litConfig):
         if litConfig.noExecute:
             return lit.Test.PASS, ''
 
         if test.config.unsupported:
             return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
         testPath, testFile = os.path.split(test.getSourcePath())
-        cmd = self.dotest_cmd + [testPath, '-p', testFile]
+        # On Windows, the system does not always correctly interpret shebang lines.
+        # To make sure we can execute the tests, add python exe as the first parameter
+        # of the command.
+        cmd = [sys.executable] + self.dotest_cmd + [testPath, '-p', testFile]
 
         try:
             out, err, exitCode = lit.util.executeCommand(
                 cmd,
                 env=test.config.environment,
                 timeout=litConfig.maxIndividualTestTime)
         except lit.util.ExecuteCommandTimeoutException:
             return (lit.Test.TIMEOUT, 'Reached timeout of {} seconds'.format(

packages/Python/lldbsuite/test/make/Makefile.rules

 SRCDIR := $(shell dirname $(firstword $(MAKEFILE_LIST)))/
 THIS_FILE_DIR := $(shell dirname $(lastword $(MAKEFILE_LIST)))/
 LLDB_BASE_DIR := $(THIS_FILE_DIR)../../../../../
 
 
 #----------------------------------------------------------------------
 # If OS is not defined, use 'uname -s' to determine the OS name.
 #
-# uname on Windows gives "windows32", but most environments standardize
-# on "Windows_NT", so we'll make it consistent here.  When running
-# tests from Visual Studio, the environment variable isn't inherited
-# all the way down to the process spawned for make.
+# uname on Windows gives "windows32" or "server version windows32", but most
+# environments standardize on "Windows_NT", so we'll make it consistent here.
+# When running tests from Visual Studio, the environment variable isn't
+# inherited all the way down to the process spawned for make.
 #----------------------------------------------------------------------
 HOST_OS = $(shell uname -s)
-ifeq "$(HOST_OS)" "windows32"
+ifneq (,$(findstring windows32,$(HOST_OS)))
   HOST_OS = Windows_NT
 endif
 ifeq "$(OS)" ""
   OS = $(HOST_OS)
 endif
 
 #----------------------------------------------------------------------
 # If TRIPLE is not defined try to set the ARCH, CC, CFLAGS, and more