Test case for the issue raised in D32271

[vbalu]

Summary

Adding test case to test the scenario in https://reviews.llvm.org/D32271

[labath]

Thank you for taking the time to add the test.

I'd like to avoid modifying the test framework for the sake of this test, if that is possible.

packages/Python/lldbsuite/test/lldbtest.py

1487 ↗

(On Diff #96676)

How about you just modify the test to create the directory and copy the file there? That way you will not have to modify the general test framework for a one-off test...

[vbalu]

Removed the changes from test frame work.

[labath]

Cool. Thank you.

[emaste]

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

43 ↗

(On Diff #96878)

whitespace issue here in raw diff

[emaste]

This test passes even without the change in D32271, presumably because exe = os.path.join(os.getcwd(), 'newdir/proc_attach') is providing us with an absolute path for argv[0].

With a change like that below the test passes with D32271 and fails without it:

os.chdir('newdir')
# Spawn a new process
popen = self.spawnSubprocess('./proc_attach')
os.chdir('..')

[vbalu]

Corrected the code. Now this test will fail without D32271 patch.

[vbalu]

modified exe path computation

[emaste]

Two small nits noted inline, but now the test does not pass without D32271 and does with it. Note that in the "without" case it returned error rather than failing, perhaps related to the path issue I noted?

Other than that issue I think this is good to go. So hopefully that can be addressed, then one of the Linux folks can confirm it's good to go. IMO it's fine for this to go in first demonstrating the bug on FreeBSD followed by a commit of D32271.

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

42 ↗

(On Diff #97575)

s/frm/from/ perhaps?

47 ↗

(On Diff #97575)

shouldn't this be ... rmtree(os.path.join(os.getcwd(), 'newdir')) instead?

[labath]

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

47 ↗

(On Diff #97575)

Probably not because he does a os.chdir(newdir) below. However, that does not make this a good idea, as you will delete an unexpected directory if the test fails before you get a chance to chdir. I like the Ed's earlier suggestion to launch the process from a separate directory more than this.

[vbalu]

Modified as suggested. sorry for the delay, i caught up in other work. Please let me know if something goes wrong.

[emaste]

Generally LGTM with two questions inline.

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

45 ↗

(On Diff #100579)

Does this one need to use os.path.join too?

53 ↗

(On Diff #100579)

Does the test infra automatically start each test in the correct directory? (I.e., chdir back to the original directory is not required?)

[emaste]

Ping for two open questions

[vbalu]

done changes as ssuggested

[emaste]

Testing this just now I got:

FAIL: LLDB (/usr/bin/cc-x86_64) :: test_attach_to_process_from_different_dir_by_id (TestProcessAttach.ProcessAttachTestCase)
======================================================================          
ERROR: test_attach_to_process_from_different_dir_by_id (TestProcessAttach.ProcessAttachTestCase)
   Test attach by process id                                                    
----------------------------------------------------------------------          
Traceback (most recent call last):                                              
  File "/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py", line 44, in test_attach_to_process_from_different_dir_by_id
    os.mkdir(os.path.join(os.getcwd(),'newdir'))                                
OSError: [Errno 17] File exists: '/tank/emaste/src/llvm/tools/lldb/packages/Python/lldbsuite/test/functionalities/process_attach/newdir'
Config=x86_64-/usr/bin/cc

investigating...

[emaste]

packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

44 ↗

(On Diff #113067)

Perhaps

try:                                                                   
    os.mkdir(os.path.join(os.getcwd(),'newdir'))                       
except OSError, e:                                                     
    if e.errno != os.errno.EEXIST:                                     
        raise

lldb/trunk/packages/Python/lldbsuite/test/functionalities/process_attach/TestProcessAttach.py

 """
 Test process attach.
 """
 
 from __future__ import print_function
 
 
 import os
 import time
 import lldb
+import shutil
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 exe_name = "ProcessAttach"  # Must match Makefile
 
 
 class ProcessAttachTestCase(TestBase):
def test_attach_to_process_by_id(self):
 
         self.runCmd("process attach -p " + str(popen.pid))
 
         target = self.dbg.GetSelectedTarget()
 
         process = target.GetProcess()
         self.assertTrue(process, PROCESS_IS_VALID)
 
+    def test_attach_to_process_from_different_dir_by_id(self):
+        """Test attach by process id"""
+        try:
+            os.mkdir(os.path.join(os.getcwd(),'newdir'))
+        except OSError, e:
+            if e.errno != os.errno.EEXIST:
+                raise
+        self.buildProgram('main.cpp',os.path.join(os.getcwd(),'newdir','proc_attach'))
+        exe = os.path.join('.','newdir','proc_attach')
+        self.addTearDownHook(lambda: shutil.rmtree(os.path.join(os.getcwd())))
+
+        # Spawn a new process
+        popen = self.spawnSubprocess(exe)
+        self.addTearDownHook(self.cleanupSubprocesses)
+
+        os.chdir('newdir')
+        self.runCmd("process attach -p " + str(popen.pid))
+
+        target = self.dbg.GetSelectedTarget()
+
+        process = target.GetProcess()
+        self.assertTrue(process, PROCESS_IS_VALID)
+
     def test_attach_to_process_by_name(self):
         """Test attach by process name"""
         self.build()
         exe = os.path.join(os.getcwd(), exe_name)
 
         # Spawn a new process
         popen = self.spawnSubprocess(exe)
         self.addTearDownHook(self.cleanupSubprocesses)