Skip to content

Skip tests if socket name is longer than 107 bytes #137405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 25, 2025

Conversation

emrekultursay
Copy link
Contributor

To fix the test failures introduced in Commit 488eeb3

@llvmbot
Copy link
Member

llvmbot commented Apr 25, 2025

@llvm/pr-subscribers-lldb

Author: Emre Kultursay (emrekultursay)

Changes

To fix the test failures introduced in Commit 488eeb3


Full diff: https://github.com/llvm/llvm-project/pull/137405.diff

1 Files Affected:

  • (modified) lldb/unittests/Host/SocketTest.cpp (+8)
diff --git a/lldb/unittests/Host/SocketTest.cpp b/lldb/unittests/Host/SocketTest.cpp
index 5bfb48b52f7e7..df67943892420 100644
--- a/lldb/unittests/Host/SocketTest.cpp
+++ b/lldb/unittests/Host/SocketTest.cpp
@@ -349,6 +349,10 @@ TEST_F(SocketTest, DomainSocketFromBoundNativeSocket) {
   ASSERT_FALSE(EC);
   llvm::sys::path::append(name, "test");
 
+  // Skip the test if the $TMPDIR is too long to hold a domain socket.
+  if (name.size() > 107u)
+    return;
+
   DomainSocket socket(true);
   Status error = socket.Listen(name, /*backlog=*/10);
   ASSERT_THAT_ERROR(error.takeError(), llvm::Succeeded());
@@ -369,6 +373,10 @@ TEST_F(SocketTest, AbstractSocketFromBoundNativeSocket) {
   llvm::sys::fs::createUniquePath("AbstractSocketFromBoundNativeSocket", name,
                                   true);
   llvm::sys::path::append(name, "test");
+  
+  // Skip the test if the $TMPDIR is too long to hold a domain socket.
+  if (name.size() > 107u)
+    return;
 
   AbstractSocket socket;
   Status error = socket.Listen(name, /*backlog=*/10);

Copy link

github-actions bot commented Apr 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

To fix the test failures introduced in Commit 488eeb3
@JDevlieghere JDevlieghere merged commit 6c78ded into llvm:main Apr 25, 2025
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 25, 2025

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building lldb at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/21023

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: Process/gdb-remote/./ProcessGdbRemoteTests/8/35 (32 of 2874)
PASS: lldb-api :: functionalities/gdb_remote_client/TestXMLRegisterFlags.py (33 of 2874)
PASS: lldb-api :: commands/gui/spawn-threads/TestGuiSpawnThreads.py (34 of 2874)
PASS: lldb-api :: tools/lldb-dap/variables/TestDAP_variables.py (35 of 2874)
PASS: lldb-api :: functionalities/recursion/TestValueObjectRecursion.py (36 of 2874)
PASS: lldb-api :: lang/c/step-target/TestStepTarget.py (37 of 2874)
PASS: lldb-api :: functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py (38 of 2874)
PASS: lldb-api :: lang/cpp/virtual-functions/TestCppVirtualFunctions.py (39 of 2874)
PASS: lldb-api :: functionalities/gdb_remote_client/TestRegDefinitionInParts.py (40 of 2874)
PASS: lldb-api :: commands/watchpoints/hello_watchlocation/TestWatchLocation.py (41 of 2874)
FAIL: lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py (42 of 2874)
******************** TEST 'lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch -p TestDAP_launch.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 6c78dedc14e7431aa0dd92b9dd8d35bed3e0ed7d)
  clang revision 6c78dedc14e7431aa0dd92b9dd8d35bed3e0ed7d
  llvm revision 6c78dedc14e7431aa0dd92b9dd8d35bed3e0ed7d
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch
runCmd: settings clear -all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output: 


Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Abstract sockets aren't tied to the filesystem, so you don't need to make them "be in" any particular directory, or even look like a path. The only reason I suggested to use createUniquePath is because it's a convenient way to create a random (path-like) string, but it looks like you're not making use of that (the first argument to this function has a slightly different meaning than it does for createUniqueDirectory).

The way I envisioned that is to do a

createUniquePath("AbstractSocketFromBoundNativeSocket-%%%%%%%%", name, /*MakeAbsolute=*/false);

.. though now that I think about it, it may be even better to detach this from the file system completely and use something like UUID::Generate().GetAsString() to generate a random name.

jyli0116 pushed a commit to jyli0116/llvm-project that referenced this pull request Apr 28, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants