This is an archive of the discontinued LLVM Phabricator instance.

Paths

Table of Contentst

-
lldb/test/API/
-
test/
-
API/
1/2
CMakeLists.txt

Differential D83767

[lldb] Use the basename of the Python test for the log name instead of the class name
AbandonedPublic

Authored by teemperor on Jul 14 2020, 6:21 AM.

Download Raw Diff

Details

Reviewers

labath
JDevlieghere

Commits

rG29aab9b5c748: [lldb] Use the basename of the Python test for the log name instead of the…

Summary

From what I know we already have the restriction that every test in the test suite needs
to have a unique file name as that's used for generating the unique build directory for
a test. It seems there is also a restriction that every test case class in the test suite needs
to have a unique name as that's used to generate the unique log file name for the test run.

This changes the log file format to use the basename of the test file instead so that we
only have to keep worrying about the 'unique file name' restriction from now on.

This came up because I started naming the test classes "TestCase" (as repeating the file
name in the test class seems like redudant information that just makes renaming tests
a pain).

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

teemperor created this revision.Jul 14 2020, 6:21 AM

Herald added subscribers: abidh, mgorny. · View Herald TranscriptJul 14 2020, 6:21 AM

I (ab)used the ability to have multiple classes in a file to run different "flavours" of an inline test in a couple of files. This will probably cause those tests to overwrite their log files, but this:
a) won't be racy because the tests within one file are run serially
b) can be fixed by giving inline log files special names, just like we already did for build directories

TBH, I'm not sure if we really need this all this flexibility with the log files. If it were up to me, I'd just put them inside the test build directories (back when this was created we were still building tests in-tree), without any fancy names and call it a day.

This revision is now accepted and ready to land.Jul 14 2020, 6:58 AM

JDevlieghere added inline comments.Jul 14 2020, 10:52 AM

lldb/test/API/CMakeLists.txt
41	Why do we override the session file format in the first place? The default in `configuration.py` is `fnmac` which should be unique.

teemperor marked an inline comment as done.Jul 14 2020, 10:53 AM

teemperor added inline comments.

lldb/test/API/CMakeLists.txt
41	The speculation in IRC was that it's because of Windows + long file names.

Closed by commit rG29aab9b5c748: [lldb] Use the basename of the Python test for the log name instead of the… (authored by teemperor). · Explain WhyJul 15 2020, 4:55 AM

This revision was automatically updated to reflect the committed changes.

Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2020, 4:55 AM

Herald added a subscriber: lldb-commits. · View Herald Transcript

Well, on Windows the file name is for some reason always "lldbsuite.test.lldbtest" so I reverted this for now. Open for suggestions.

This revision is now accepted and ready to land.Jul 15 2020, 7:28 AM

In D83767#2153236, @teemperor wrote:

Well, on Windows the file name is for some reason always "lldbsuite.test.lldbtest" so I reverted this for now. Open for suggestions.

I'm on board with Pavel's suggestion of getting rid of the session dir and format and just adding the log to the build dir.

Got reverted, so back to my TODO queue.

teemperor mentioned this in D85141: [lldb] Enable std::pair in CxxModuleHandler.Sep 7 2020, 1:49 AM

teemperor mentioned this in D92425: [lldb] Don't reject empty or unnamed template parameter packs.Dec 2 2020, 10:26 AM

Abandoned in favor of D92498

teemperor mentioned this in rGe97b991eef63: [lldb] Remove LLDB session dir and just store test traces in the respective….Dec 4 2020, 2:43 AM

Revision Contents

Path

Size

lldb/

test/

API/

CMakeLists.txt

2 lines

Diff 278145

lldb/test/API/CMakeLists.txt

	Show All 32 Lines
	# The .noindex suffix is a marker for Spotlight to never index the			# The .noindex suffix is a marker for Spotlight to never index the
	# build directory. LLDB queries Spotlight to locate .dSYM bundles			# build directory. LLDB queries Spotlight to locate .dSYM bundles
	# based on the UUID embedded in a binary, and because the UUID is a			# based on the UUID embedded in a binary, and because the UUID is a
	# hash of filename and .text section, there will be conflicts inside			# hash of filename and .text section, there will be conflicts inside
	# the build directory.			# the build directory.
	set(LLDB_TEST_COMMON_ARGS			set(LLDB_TEST_COMMON_ARGS
	-s			-s
	${CMAKE_BINARY_DIR}/lldb-test-traces			${CMAKE_BINARY_DIR}/lldb-test-traces
	-S nm			-S fm
				JDevlieghereUnsubmitted Not Done Reply Inline Actions Why do we override the session file format in the first place? The default in `configuration.py` is `fnmac` which should be unique. JDevlieghere: Why do we override the session file format in the first place? The default in `configuration.
				teemperorAuthorUnsubmitted Done Reply Inline Actions The speculation in IRC was that it's because of Windows + long file names. teemperor: The speculation in IRC was that it's because of Windows + long file names.
	-u CXXFLAGS			-u CXXFLAGS
	-u CFLAGS			-u CFLAGS
	)			)

	# Set the path to the default lldb test executable.			# Set the path to the default lldb test executable.
	set(LLDB_DEFAULT_TEST_EXECUTABLE "${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")			set(LLDB_DEFAULT_TEST_EXECUTABLE "${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")

	# Set the paths to default llvm tools.			# Set the paths to default llvm tools.
	▲ Show 20 Lines • Show All 144 Lines • Show Last 20 Lines