Fix ndk-stack, remove test reliance on mocks.

ndk-stack was moved to being a zipapp, which changed the path of
__file__ relative to the root of the NDK.

This was covered by a test (97% line coverage, even), but the test was
useless because it was only testing mocks, and the mocks weren't
updated.

The "system test" has been updated to test the real ndk-stack entry
point in the NDK rather than importing the source and mocking dependency
discovery. This unfortunately means that running the tests now depends
on more or less the whole NDK, but that's true of everything else in the
tests/ directory anyway. If that's ever a problem, we should split up
the tests so that the test of the NDK itself and the tests of the ndk
python package are kept separately.

Bug: android/ndk#1751
Test: fixed the test, also tested manually
(cherry picked from https://android-review.googlesource.com/q/commit:a54b719d892e8d19fe0ee9218762f0d9093c88f1)
Merged-In: I77076d1c3fc8ac5a05a93dbe1ce3488874283143
Change-Id: I77076d1c3fc8ac5a05a93dbe1ce3488874283143

Author: DanAlbert

docs/changelogs/Changelog-r26.md

@@ -21,6 +21,13 @@ directly, see the [build system maintainers guide].
 
 [Issue 1751]: https://github.com/android/ndk/issues/1751
 
+## r26b
+
+* [Issue 1938]: Fixed ndk-stack to use the correct path for llvm-symbolizer and
+  other tools.
+
+[Issue 1938]: https://github.com/android/ndk/issues/1938
+
 ## Changes
 
 * Updated LLVM to clang-r487747c, based on LLVM 17 development.

ndk/checkbuild.py

@@ -845,6 +845,7 @@ def run(self) -> None:
 @register
 class Pytest(ndk.builds.LintModule):
     name = "pytest"
+    deps = {"ndk-stack", "ndk-stack-shortcut"}
 
     def run(self) -> None:
         if not shutil.which("pytest"):
@@ -1781,7 +1782,13 @@ class NdkStack(ndk.builds.PythonApplication):
     notice = NDK_DIR / "NOTICE"
     package = NDK_DIR / "ndkstack.py"
     main = "ndkstack:main"
-    deps = {"ndk-stack-shortcut"}
+    deps = {
+        # PythonApplication depends on build/tools/ndk_bin_common.sh.
+        "ndk-build",
+        "ndk-stack-shortcut",
+        # PythonApplication depends on Python, which is bundled with Clang.
+        "toolchain",
+    }
 
 
 @register

ndkstack.py

@@ -28,6 +28,7 @@
 import sys
 import tempfile
 import zipfile
+from pathlib import Path
 
 EXE_SUFFIX = ".exe" if os.name == "nt" else ""
 
@@ -57,14 +58,22 @@ def get_ndk_paths() -> tuple[str, str, str]:
              The platform name (eg linux-x86_64).
     """
 
+    # ndk-stack is installed as a zipped Python application (created with zipapp). The
+    # behavior of __file__ when Python runs a zip file doesn't appear to be documented,
+    # but experimentally for this case it will be:
+    #
+    #     $NDK/prebuilt/darwin-x86_64/bin/ndkstack.pyz/ndkstack.py
+    #
     # ndk-stack is installed to $NDK/prebuilt/<platform>/bin, so from
     # `android-ndk-r18/prebuilt/linux-x86_64/bin/ndk-stack`...
     # ...get `android-ndk-r18/`:
-    ndk_bin = os.path.dirname(os.path.realpath(__file__))
-    ndk_root = os.path.abspath(os.path.join(ndk_bin, "../../.."))
+    path_in_zipped_app = Path(__file__)
+    zip_root = path_in_zipped_app.parent
+    ndk_bin = zip_root.parent
+    ndk_root = ndk_bin.parent.parent.parent
     # ...get `linux-x86_64`:
-    ndk_host_tag = os.path.basename(os.path.abspath(os.path.join(ndk_bin, "../")))
-    return (ndk_root, ndk_bin, ndk_host_tag)
+    ndk_host_tag = ndk_bin.parent.name
+    return (str(ndk_root), str(ndk_bin), str(ndk_host_tag))
 
 
 def find_llvm_symbolizer(ndk_root: str, ndk_bin: str, ndk_host_tag: str) -> str:

tests/pytest/ndkstack/test_systemtest.py

@@ -16,77 +16,59 @@
 #
 """System tests for ndk-stack.py"""
 
-from __future__ import print_function
-
 import os.path
+import subprocess
 import unittest
-from io import StringIO
-from unittest.mock import patch
 
-import ndk.hosts
+import ndk.paths
 import ndk.toolchains
-import ndkstack
+from ndk.hosts import Host
 
 
 class SystemTests(unittest.TestCase):
     """Complete system test of ndk-stack.py script."""
 
-    def setUp(self):
-        default_host = ndk.hosts.get_default_host()
-        clang_toolchain = ndk.toolchains.ClangToolchain(default_host)
-
-        # First try and use the normal functions, and if they fail, then
-        # use hard-coded paths from the development locations.
-        ndk_paths = ndkstack.get_ndk_paths()
-        self.readelf = ndkstack.find_readelf(*ndk_paths)
-        if not self.readelf:
-            self.readelf = clang_toolchain.clang_tool("llvm-readelf")
-        self.assertTrue(self.readelf)
-        self.assertTrue(os.path.exists(self.readelf))
-
-        try:
-            self.llvm_symbolizer = ndkstack.find_llvm_symbolizer(*ndk_paths)
-        except OSError:
-            self.llvm_symbolizer = str(clang_toolchain.clang_tool("llvm-symbolizer"))
-        self.assertTrue(self.llvm_symbolizer)
-        self.assertTrue(os.path.exists(self.llvm_symbolizer))
+    def system_test(self, backtrace_file: str, expected_file: str) -> None:
+        ndk_path = ndk.paths.get_install_path()
+        self.assertTrue(
+            ndk_path.exists(),
+            f"{ndk_path} does not exist. Build the NDK before running this test.",
+        )
 
-    @patch.object(ndkstack, "find_llvm_symbolizer")
-    @patch.object(ndkstack, "find_readelf")
-    def system_test(
-        self, backtrace_file, expected_file, mock_readelf, mock_llvm_symbolizer
-    ):
-        mock_readelf.return_value = self.readelf
-        mock_llvm_symbolizer.return_value = self.llvm_symbolizer
+        ndk_stack = ndk_path / "ndk-stack"
+        if Host.current() is Host.Windows64:
+            ndk_stack = ndk_stack.with_suffix(".bat")
 
         symbol_dir = os.path.join(os.path.dirname(os.path.realpath(__file__)), "files")
-        with patch("sys.stdout", new_callable=StringIO) as mock_stdout:
-            ndkstack.main(
-                ["-s", symbol_dir, "-i", os.path.join(symbol_dir, backtrace_file)]
-            )
+        proc = subprocess.run(
+            [
+                ndk_stack,
+                "-s",
+                symbol_dir,
+                "-i",
+                os.path.join(symbol_dir, backtrace_file),
+            ],
+            check=True,
+            capture_output=True,
+            text=True,
+        )
 
         # Read the expected output.
         file_name = os.path.join(symbol_dir, expected_file)
         with open(file_name, "r", encoding="utf-8") as exp_file:
             expected = exp_file.read()
         expected = expected.replace("SYMBOL_DIR", symbol_dir)
         self.maxDiff = None
-        self.assertEqual(expected, mock_stdout.getvalue())
+        self.assertEqual(expected, proc.stdout)
 
-    def test_all_stacks(self):
-        self.system_test(  # pylint:disable=no-value-for-parameter
-            "backtrace.txt", "expected.txt"
-        )
+    def test_all_stacks(self) -> None:
+        self.system_test("backtrace.txt", "expected.txt")
 
-    def test_multiple_crashes(self):
-        self.system_test(  # pylint:disable=no-value-for-parameter
-            "multiple.txt", "expected_multiple.txt"
-        )
+    def test_multiple_crashes(self) -> None:
+        self.system_test("multiple.txt", "expected_multiple.txt")
 
-    def test_hwasan(self):
-        self.system_test(  # pylint:disable=no-value-for-parameter
-            "hwasan.txt", "expected_hwasan.txt"
-        )
+    def test_hwasan(self) -> None:
+        self.system_test("hwasan.txt", "expected_hwasan.txt")
 
 
 if __name__ == "__main__":