Allow custom formatting of session log file names

[zturner]

Summary

I'm trying to get the LLDB buildbot to run tests, and currently the issue is that many tests fail when trying to write the session log file. The build directory on the buildbot is already nested kind of deep, which means the path to clang.exe is also deep, and having this in the filename results in paths that are 400+ characters long, which don't work well in Windows.

To fix this, I'm allowing the specification of a custom file format, where you can specify various fields to be included in the filename. The default behavior of dotest is unchanged, it will still display everything it's always displayed. But if you're using CMake then the default will be to not include the compiler path. This is because by default CMake also uses only runs tests with one compiler anyway, so the compiler path is not needed to distinguish identical tests run with different compilers.

[tfiala]

Hi Zachary,

I'll have a look at this first thing in the morning!

-Todd

[tfiala]

Okay I had a look now. Just one comment on the code (inline), just for consideration. Other than that, LGTM.

packages/Python/lldbsuite/test/dotest_args.py

73 ↗

(On Diff #57409)

group.add_argument('-S', metavar='format', help='Specify session file name format. See configuration.py for a description. Default="fnmac"')

Maybe include a long-style arg name as well? Something like:

group.add_argument('--session-file-format', '-S', metavar='format', help='Specify session file name format. See configuration.py for a description. Default="fnmac"')

Then it could be referenced as "if args.session_file_format" instead of "if args.S". But users could still use either '--session-file-format' or '-S' on the command line.

Not a big deal, but more readable for future maintainability, particularly if these options get ensconced in other CI scripts and the like, where a longer name is more helpful to see.

[labath]

I agree with Todd that we should have a long option for this, as short ones tend to be cryptic. If we don't anticipate using this often (I don't), we could even drop the short option altogether, as it's only advantage is that it is shorter to type. I'll leave that up to you, no need to go through review second time.

test/CMakeLists.txt

30 ↗

(On Diff #57409)

If you don't intend to use this functionality, I wouldn't be opposed to just hardcoding the default. Any advanced users needing to set this will probably invoke dotest directly. Doesn't really matter though...

[tfiala]

(Changing to Accept - although please have a look at the comments from Pavel and me).

lldb/trunk/packages/Python/lldbsuite/test/configuration.py

 # The regular expression pattern to match against eligible filenames as our test cases.
 regexp = None
 
 # By default, recorded session info for errored/failed test are dumped into its
 # own file under a session directory named after the timestamp of the test suite
 # run.  Use '-s session-dir-name' to specify a specific dir name.
 sdir_name = None
 
+# Valid options:
+# f - test file name (without extension)
+# n - test class name
+# m - test method name
+# a - architecture
+# c - compiler path
+# The default is to write all fields.
+session_file_format = 'fnmac'
+
 # Set this flag if there is any session info dumped during the test run.
 sdir_has_content = False
 
 # svn_info stores the output from 'svn info lldb.base.dir'.
 svn_info = ''
 
 # Default verbosity is 0.
 verbose = 0

lldb/trunk/packages/Python/lldbsuite/test/dosep.py

for (base_name, full_test_path) in files:
         import __main__ as main
         global dotest_options
         if dotest_options.p and not re.search(dotest_options.p, base_name):
             continue
 
         script_file = main.__file__
         command = ([sys.executable, script_file] +
                    dotest_argv +
+                   ["-S", dotest_options.session_file_format] +
                    ["--inferior", "-p", base_name, root])
 
         timeout_name = os.path.basename(os.path.splitext(base_name)[0]).upper()
 
         timeout = (os.getenv("LLDB_%s_TIMEOUT" % timeout_name) or
                    getDefaultTimeout(dotest_options.lldb_platform_name))
 
         results.append(call_with_timeout(

lldb/trunk/packages/Python/lldbsuite/test/dotest.py

def parseOptionsAndInitTestdirs():
 
     if args.q:
         configuration.parsable = True
 
     if args.s:
         if args.s.startswith('-'):
             usage(parser)
         configuration.sdir_name = args.s
+    configuration.session_file_format = args.session_file_format
 
     if args.t:
         os.environ['LLDB_COMMAND_TRACE'] = 'YES'
 
     if args.v:
         configuration.verbose = 2
 
     # argparse makes sure we have a number

lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py

 from __future__ import print_function
 from __future__ import absolute_import
 
 # System modules
 import argparse
 import sys
 import multiprocessing
 import os
 import textwrap
 
 # Third-party modules
 
 # LLDB modules
+from . import configuration
 
 class ArgParseNamespace(object):
     pass
 
 def parse_args(parser, argv):
     """ Returns an argument object. LLDB_TEST_ARGUMENTS environment variable can
         be used to pass additional arguments.
     """
def create_parser():
     group.add_argument('-G', '--category', metavar='category', action='append', dest='categoriesList', help=textwrap.dedent('''Specify categories of test cases of interest. Can be specified more than once.'''))
     group.add_argument('--skip-category', metavar='category', action='append', dest='skipCategories', help=textwrap.dedent('''Specify categories of test cases to skip. Takes precedence over -G. Can be specified more than once.'''))
 
     # Configuration options
     group = parser.add_argument_group('Configuration options')
     group.add_argument('--framework', metavar='framework-path', help='The path to LLDB.framework')
     group.add_argument('--executable', metavar='executable-path', help='The path to the lldb executable')
     group.add_argument('-s', metavar='name', help='Specify the name of the dir created to store the session files of tests with errored or failed status. If not specified, the test driver uses the timestamp as the session dir name')
+    group.add_argument('-S', '--session-file-format', default=configuration.session_file_format, metavar='format', help='Specify session file name format.  See configuration.py for a description.')
     group.add_argument('-y', type=int, metavar='count', help="Specify the iteration count used to collect our benchmarks. An example is the number of times to do 'thread step-over' to measure stepping speed.")
     group.add_argument('-#', type=int, metavar='sharp', dest='sharp', help='Repeat the test suite for a specified number of times')
     group.add_argument('--channel', metavar='channel', dest='channels', action='append', help=textwrap.dedent("Specify the log channels (and optional categories) e.g. 'lldb all' or 'gdb-remote packets' if no categories are specified, 'default' is used"))
     group.add_argument('--log-success', dest='log_success', action='store_true', help="Leave logs/traces even for successful test runs (useful for creating reference log files during debugging.)")
 
     # Configuration options
     group = parser.add_argument_group('Remote platform options')
     group.add_argument('--platform-name', dest='lldb_platform_name', metavar='platform-name', help='The name of a remote platform to use')

lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

def getLogBasenameForCurrentTest(self, prefix=None):
 
         <session-dir>/<arch>-<compiler>-<test-file>.<test-class>.<test-method>
         """
         dname = os.path.join(os.environ["LLDB_TEST"],
                      os.environ["LLDB_SESSION_DIRNAME"])
         if not os.path.isdir(dname):
             os.mkdir(dname)
 
+        components = []
+        if prefix is not None:
+            components.append(prefix)
+        for c in configuration.session_file_format:
+            if c == 'f':
+                components.append(self.__class__.__module__)
+            elif c == 'n':
+                components.append(self.__class__.__name__)
+            elif c == 'c':
                 compiler = self.getCompiler()
 
                 if compiler[1] == ':':
                     compiler = compiler[2:]
                 if os.path.altsep is not None:
                     compiler = compiler.replace(os.path.altsep, os.path.sep)
-
-        fname = "{}-{}-{}".format(self.id(), self.getArchitecture(), "_".join(compiler.split(os.path.sep)))
-        if len(fname) > 200:
-            fname = "{}-{}-{}".format(self.id(), self.getArchitecture(), compiler.split(os.path.sep)[-1])
-
-        if prefix is not None:
-            fname = "{}-{}".format(prefix, fname)
+                components.extend([x for x in compiler.split(os.path.sep) if x != ""])
+            elif c == 'a':
+                components.append(self.getArchitecture())
+            elif c == 'm':
+                components.append(self.testMethodName)
+        fname = "-".join(components)
 
         return os.path.join(dname, fname)
 
     def dumpSessionInfo(self):
         """
         Dump the debugger interactions leading to a test error/failure.  This
         allows for more convenient postmortem analysis.
 

lldb/trunk/test/CMakeLists.txt

set(LLDB_TEST_USER_ARGS
   CACHE STRING "Specify additional arguments to pass to test runner. For example: '-C gcc -C clang -A i386 -A x86_64'")
 
 set(LLDB_TEST_COMMON_ARGS
   --arch=${LLDB_TEST_ARCH}
   --executable
   ${CMAKE_BINARY_DIR}/bin/lldb${CMAKE_EXECUTABLE_SUFFIX}
   -s
   ${CMAKE_BINARY_DIR}/lldb-test-traces
+  -S nm
   -u CXXFLAGS
   -u CFLAGS
   )
 
 if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )
   set(LLDB_TEST_DEBUG_TEST_CRASHES
     0
     CACHE BOOL "(Windows only) Enables debugging of tests in the test suite by showing the crash dialog when lldb crashes")