[Lldb-commits] [lldb] [lldb] Fix and re-enable TestUseSourceCache.py (PR #111237)

Igor Kudrin via lldb-commits lldb-commits at lists.llvm.org
Thu Oct 10 15:08:58 PDT 2024


https://github.com/igorkudrin updated https://github.com/llvm/llvm-project/pull/111237

>From 6756842b1c78ac6f41fa467f112aa7632f260582 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Fri, 4 Oct 2024 23:27:04 -0700
Subject: [PATCH 1/3] [lldb] Fix and re-enable TestUseSourceCache.py

The decorators caused the main test, i.e. `test_set_use_source_cache_true()`,
to be skipped in most scenarios. In fact, it was only run on a Windows host
targeting a non-Windows remote platform, and it failed in this case because
the source file is opened with the `FILE_SHARE_DELETE` share mode, which
allows the file to be removed, see `llvm::sys::fs::openNativeFileInternal()`
in `llvm/lib/Support/Windows/Path.inc`.

This patch changes the test to check that the content can still be displayed
even after deleting the file when caching is enabled. The updated test is
expected to work on any platform, so the decorators are removed.
---
 .../settings/use_source_cache/TestUseSourceCache.py   | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py b/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
index 421599080a9e51..421b13f253f05b 100644
--- a/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
+++ b/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
@@ -17,8 +17,6 @@ def test_set_use_source_cache_false(self):
         """Test that after 'set use-source-cache false', files are not locked."""
         self.set_use_source_cache_and_test(False)
 
-    @skipIf(hostoslist=no_match(["windows"]))
-    @skipIf(oslist=["windows"])  # Fails on windows 11
     def test_set_use_source_cache_true(self):
         """Test that after 'set use-source-cache false', files are locked."""
         self.set_use_source_cache_and_test(True)
@@ -43,16 +41,15 @@ def set_use_source_cache_and_test(self, is_cache_enabled):
             self, "calc"
         )
 
-        # Show the source file contents to make sure LLDB loads src file.
-        self.runCmd("source list")
+        # Ensure that the source file is loaded.
+        self.expect("step", patterns=["-> .+ int x4 ="])
 
         # Try deleting the source file.
         is_file_removed = self.removeFile(src)
 
         if is_cache_enabled:
-            self.assertFalse(
-                is_file_removed, "Source cache is enabled, but delete file succeeded"
-            )
+            # Regardless of whether the file is removed, its contents should be displayed.
+            self.expect("step", patterns=["-> .+ int x5 ="])
 
         if not is_cache_enabled:
             self.assertTrue(

>From ef848fd2c7de9483cfe2cb0286a20df0a2005f97 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Tue, 8 Oct 2024 22:16:23 -0700
Subject: [PATCH 2/3] `removeFile()` -> `overwriteFile()`

---
 .../use_source_cache/TestUseSourceCache.py    | 26 +++++++++++--------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py b/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
index 421b13f253f05b..0c005ff3ab21b1 100644
--- a/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
+++ b/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
@@ -17,8 +17,9 @@ def test_set_use_source_cache_false(self):
         """Test that after 'set use-source-cache false', files are not locked."""
         self.set_use_source_cache_and_test(False)
 
+    @skipIf(hostoslist=no_match(["windows"]))
     def test_set_use_source_cache_true(self):
-        """Test that after 'set use-source-cache false', files are locked."""
+        """Test that after 'set use-source-cache true', files are locked."""
         self.set_use_source_cache_and_test(True)
 
     def set_use_source_cache_and_test(self, is_cache_enabled):
@@ -41,25 +42,28 @@ def set_use_source_cache_and_test(self, is_cache_enabled):
             self, "calc"
         )
 
-        # Ensure that the source file is loaded.
-        self.expect("step", patterns=["-> .+ int x4 ="])
+        # Show the source file contents to make sure LLDB loads src file.
+        self.runCmd("source list")
 
-        # Try deleting the source file.
-        is_file_removed = self.removeFile(src)
+        # Try overwriting the source file.
+        is_file_overwritten = self.overwriteFile(src)
 
         if is_cache_enabled:
-            # Regardless of whether the file is removed, its contents should be displayed.
-            self.expect("step", patterns=["-> .+ int x5 ="])
+            self.assertFalse(
+                is_file_overwritten, "Source cache is enabled, but writing to file succeeded"
+            )
 
         if not is_cache_enabled:
             self.assertTrue(
-                is_file_removed, "Source cache is disabled, but delete file failed"
+                is_file_overwritten, "Source cache is disabled, but writing to file failed"
             )
 
-    def removeFile(self, src):
-        """Remove file and return true iff file was successfully removed."""
+    def overwriteFile(self, src):
+        """Write to file and return true iff file was successfully written."""
         try:
-            os.remove(src)
+            f = open(src, "w")
+            f.writelines(["// hello world\n"])
+            f.close()
             return True
         except Exception:
             return False

>From 59d1ba9e785f58f276234a0a82b2b3acf3340733 Mon Sep 17 00:00:00 2001
From: Igor Kudrin <ikudrin at accesssoftek.com>
Date: Thu, 10 Oct 2024 15:08:32 -0700
Subject: [PATCH 3/3] Make the Python code formatter happy

---
 .../settings/use_source_cache/TestUseSourceCache.py         | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py b/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
index 0c005ff3ab21b1..9ed57afd41077b 100644
--- a/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
+++ b/lldb/test/API/commands/settings/use_source_cache/TestUseSourceCache.py
@@ -50,12 +50,14 @@ def set_use_source_cache_and_test(self, is_cache_enabled):
 
         if is_cache_enabled:
             self.assertFalse(
-                is_file_overwritten, "Source cache is enabled, but writing to file succeeded"
+                is_file_overwritten,
+                "Source cache is enabled, but writing to file succeeded"
             )
 
         if not is_cache_enabled:
             self.assertTrue(
-                is_file_overwritten, "Source cache is disabled, but writing to file failed"
+                is_file_overwritten,
+                "Source cache is disabled, but writing to file failed"
             )
 
     def overwriteFile(self, src):



More information about the lldb-commits mailing list