[Lldb-commits] [PATCH] D85258: [test] Use realpath consistently for test root file paths.

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
rupprecht marked an inline comment as done.
Closed by commit rGfcb0d8163a4f: [lldb/test] Use realpath consistently for test 
root file paths. (authored by rupprecht).

Changed prior to commit:
  https://reviews.llvm.org/D85258?vs=283060=283316#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85258/new/

https://reviews.llvm.org/D85258

Files:
  lldb/packages/Python/lldbsuite/__init__.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/use_lldb_suite.py


Index: lldb/test/API/use_lldb_suite.py
===
--- lldb/test/API/use_lldb_suite.py
+++ lldb/test/API/use_lldb_suite.py
@@ -4,9 +4,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(
-os.path.abspath(inspect.getfile(inspect.currentframe()))
-)
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -496,8 +496,12 @@
 mydir = TestBase.compute_mydir(__file__)
 '''
 # /abs/path/to/packages/group/subdir/mytest.py -> group/subdir
-rel_prefix = test_file[len(configuration.test_src_root) + 1:]
-return os.path.dirname(rel_prefix)
+lldb_test_src = configuration.test_src_root
+if not test_file.startswith(lldb_test_src):
+  raise Exception(
+  "Test file '%s' must reside within lldb_test_src "
+  "(which is '%s')." % (test_file, lldb_test_src))
+return os.path.dirname(os.path.relpath(test_file, start=lldb_test_src))
 
 def TraceOn(self):
 """Returns True if we are in trace mode (tracing detailed test 
execution)."""
Index: lldb/packages/Python/lldbsuite/__init__.py
===
--- lldb/packages/Python/lldbsuite/__init__.py
+++ lldb/packages/Python/lldbsuite/__init__.py
@@ -6,7 +6,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(inspect.getfile(inspect.currentframe()))
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'


Index: lldb/test/API/use_lldb_suite.py
===
--- lldb/test/API/use_lldb_suite.py
+++ lldb/test/API/use_lldb_suite.py
@@ -4,9 +4,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(
-os.path.abspath(inspect.getfile(inspect.currentframe()))
-)
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -496,8 +496,12 @@
 mydir = TestBase.compute_mydir(__file__)
 '''
 # /abs/path/to/packages/group/subdir/mytest.py -> group/subdir
-rel_prefix = test_file[len(configuration.test_src_root) + 1:]
-return os.path.dirname(rel_prefix)
+lldb_test_src = configuration.test_src_root
+if not test_file.startswith(lldb_test_src):
+  raise Exception(
+  "Test file '%s' must reside within lldb_test_src "
+  "(which is '%s')." % (test_file, lldb_test_src))
+return os.path.dirname(os.path.relpath(test_file, start=lldb_test_src))
 
 def TraceOn(self):
 """Returns True if we are in trace mode (tracing detailed test execution)."""
Index: lldb/packages/Python/lldbsuite/__init__.py
===
--- lldb/packages/Python/lldbsuite/__init__.py
+++ lldb/packages/Python/lldbsuite/__init__.py
@@ -6,7 +6,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(inspect.getfile(inspect.currentframe()))
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D85258: [test] Use realpath consistently for test root file paths.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht marked an inline comment as done.
rupprecht added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:499
 # /abs/path/to/packages/group/subdir/mytest.py -> group/subdir
-rel_prefix = test_file[len(os.environ["LLDB_TEST_SRC"]) + 1:]
-return os.path.dirname(rel_prefix)
+lldb_test_src = os.environ["LLDB_TEST_SRC"]
+if not test_file.startswith(lldb_test_src):

JDevlieghere wrote:
> While you are here... can you change this to pass the source directory trough 
> the `configuration`?
Absolutely, but in the spirit of small/isolated changes, split off as D85322.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85258/new/

https://reviews.llvm.org/D85258

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D85258: [test] Use realpath consistently for test root file paths.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:499
 # /abs/path/to/packages/group/subdir/mytest.py -> group/subdir
-rel_prefix = test_file[len(os.environ["LLDB_TEST_SRC"]) + 1:]
-return os.path.dirname(rel_prefix)
+lldb_test_src = os.environ["LLDB_TEST_SRC"]
+if not test_file.startswith(lldb_test_src):

While you are here... can you change this to pass the source directory trough 
the `configuration`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85258/new/

https://reviews.llvm.org/D85258

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D85258: [test] Use realpath consistently for test root file paths.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
rupprecht requested review of this revision.
Herald added a subscriber: JDevlieghere.

LLDB tests assume that tests are in the test tree (the `LLDB_TEST_SRC` env 
variable, configured by `dotest.py`).
If this assertion doesn't hold, tests fail in strange ways. An early place this 
goes wrong is in `compute_mydir` which does a simple length-based substring to 
get the relative path. Later, we use that path to chdir to. If the test file 
and test tree don't agree in realpath-ness (and therefore length), this will be 
a cryptic error of chdir-ing to a directory that does not exist.

The actual discrepency is that the places we look for `use_lldb_suite.py` don't 
use a realpath, but `dotest.py` does (see initialization of 
`configuration.testdirs`).

It doesn't particularly matter whether we use realpath or abspath to 
canonicalize things, but many places end up with implicit dependencies on the 
canonicalized pwd being a realpath, so make them realpath consistently. Also, 
in the `compute_mydir` method mentioned, raise an error if the path types don't 
agree.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85258

Files:
  lldb/packages/Python/lldbsuite/__init__.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/test/API/use_lldb_suite.py


Index: lldb/test/API/use_lldb_suite.py
===
--- lldb/test/API/use_lldb_suite.py
+++ lldb/test/API/use_lldb_suite.py
@@ -4,9 +4,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(
-os.path.abspath(inspect.getfile(inspect.currentframe()))
-)
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -496,8 +496,12 @@
 mydir = TestBase.compute_mydir(__file__)
 '''
 # /abs/path/to/packages/group/subdir/mytest.py -> group/subdir
-rel_prefix = test_file[len(os.environ["LLDB_TEST_SRC"]) + 1:]
-return os.path.dirname(rel_prefix)
+lldb_test_src = os.environ["LLDB_TEST_SRC"]
+if not test_file.startswith(lldb_test_src):
+  raise Exception(
+  "Test file '%s' must reside within LLDB_TEST_SRC "
+  "(which is '%s')." % (test_file, lldb_test_src))
+return os.path.dirname(os.path.relpath(test_file, start=lldb_test_src))
 
 def TraceOn(self):
 """Returns True if we are in trace mode (tracing detailed test 
execution)."""
Index: lldb/packages/Python/lldbsuite/__init__.py
===
--- lldb/packages/Python/lldbsuite/__init__.py
+++ lldb/packages/Python/lldbsuite/__init__.py
@@ -6,7 +6,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(inspect.getfile(inspect.currentframe()))
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'


Index: lldb/test/API/use_lldb_suite.py
===
--- lldb/test/API/use_lldb_suite.py
+++ lldb/test/API/use_lldb_suite.py
@@ -4,9 +4,8 @@
 
 
 def find_lldb_root():
-lldb_root = os.path.dirname(
-os.path.abspath(inspect.getfile(inspect.currentframe()))
-)
+lldb_root = os.path.realpath(
+os.path.dirname(inspect.getfile(inspect.currentframe(
 while True:
 parent = os.path.dirname(lldb_root)
 if parent == lldb_root: # dirname('/') == '/'
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -496,8 +496,12 @@
 mydir = TestBase.compute_mydir(__file__)
 '''
 # /abs/path/to/packages/group/subdir/mytest.py -> group/subdir
-rel_prefix = test_file[len(os.environ["LLDB_TEST_SRC"]) + 1:]
-return os.path.dirname(rel_prefix)
+lldb_test_src = os.environ["LLDB_TEST_SRC"]
+if not test_file.startswith(lldb_test_src):
+  raise Exception(
+  "Test file '%s' must reside within LLDB_TEST_SRC "
+  "(which is '%s')." % (test_file, lldb_test_src))
+return os.path.dirname(os.path.relpath(test_file, start=lldb_test_src))
 
 def TraceOn(self):
 """Returns True if we are in trace mode (tracing detailed test execution)."""
Index: