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

[Igor Kudrin via lldb-commits]

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/2] [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/2] `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