Adding test case to test the scenario in https://reviews.llvm.org/D32271
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.
|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...
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.
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('..')
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.
|42 ↗||(On Diff #97575)|
|47 ↗||(On Diff #97575)|
shouldn't this be ... rmtree(os.path.join(os.getcwd(), 'newdir')) instead?
|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.
Generally LGTM with two questions inline.
|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?)
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
|44 ↗||(On Diff #113067)|
try: os.mkdir(os.path.join(os.getcwd(),'newdir')) except OSError, e: if e.errno != os.errno.EEXIST: raise