-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[lldb] Fix API test for file redirection to existing files #114119
Conversation
@llvm/pr-subscribers-lldb Author: Wanyi (kusmour) ChangesAPI test failed for remote debugging in #112657 Fixing it properly Full diff: https://github.com/llvm/llvm-project/pull/114119.diff 1 Files Affected:
diff --git a/lldb/test/API/python_api/process/io/TestProcessIO.py b/lldb/test/API/python_api/process/io/TestProcessIO.py
index 3b5c7c48c51f4d..24cd845904cc2c 100644
--- a/lldb/test/API/python_api/process/io/TestProcessIO.py
+++ b/lldb/test/API/python_api/process/io/TestProcessIO.py
@@ -99,31 +99,45 @@ def test_stdout_stderr_redirection(self):
@expectedFlakeyLinux(bugnumber="llvm.org/pr26437")
@skipIfDarwinEmbedded # debugserver can't create/write files on the device
def test_stdout_stderr_redirection_to_existing_files(self):
- """Exercise SBLaunchInfo::AddOpenFileAction() for STDOUT and STDERR without redirecting STDIN to output files already exist."""
+ """Exercise SBLaunchInfo::AddOpenFileAction() for STDOUT and STDERR redirect to output files already exist."""
self.setup_test()
self.build()
self.create_target()
- self.write_file_with_placeholder(self.output_file)
- self.write_file_with_placeholder(self.error_file)
- self.redirect_stdout()
- self.redirect_stderr()
- self.run_process(True)
- output = self.read_output_file_and_delete()
- error = self.read_error_file_and_delete()
- self.check_process_output(output, error)
- def write_file_with_placeholder(self, target_file):
+ # Create the output and error files with placeholder
placeholder = "This content should be overwritten."
if lldb.remote_platform:
+ f = open(self.local_output_file, "w")
+ f.write(placeholder)
+ f.close()
+
+ f = open(self.local_error_file, "w")
+ f.write(placeholder)
+ f.close()
+ self.runCmd(
+ 'platform put-file "{local}" "{remote}"'.format(
+ local=self.local_output_file, remote=self.output_file
+ )
+ )
self.runCmd(
- 'platform file write "{target}" -d "{data}"'.format(
- target=target_file, data=placeholder
+ 'platform put-file "{local}" "{remote}"'.format(
+ local=self.local_error_file, remote=self.error_file
)
)
else:
- f = open(target_file, "w")
+ f = open(self.output_file, "w")
+ f.write(placeholder)
+ f.close()
+ f = open(self.error_file, "w")
f.write(placeholder)
f.close()
+
+ self.redirect_stdout()
+ self.redirect_stderr()
+ self.run_process(True)
+ output = self.read_output_file_and_delete()
+ error = self.read_error_file_and_delete()
+ self.check_process_output(output, error)
# target_file - path on local file system or remote file system if running remote
# local_file - path on local system
|
✅ With the latest revision this PR passed the Python code formatter. |
placeholder = "This content should be overwritten." | ||
if lldb.remote_platform: | ||
f = open(self.local_output_file, "w") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this create files on your local filesystem now instead of doing the previous platform file write
commands? Was there something wrong with the previous approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going to make local files the Save core tests put them in the build directory as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I double checked the platform file write
it will require platform file open
first and obtain the file descriptor so that we can write.
The put method however won't require extra steps and is used in the same test already.
placeholder = "This content should be overwritten." | ||
if lldb.remote_platform: | ||
f = open(self.local_output_file, "w") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going to make local files the Save core tests put them in the build directory as an example.
dfa3252
to
a1b19df
Compare
API test failed for remote platform in #112657
See test detail: https://lab.llvm.org/buildbot/#/builders/195/builds/321
Previously when putting files onto remote platform, I used
platform file write -d <data>
which actually required aplatform file open <path>
first in order to obtain a file descriptor.eg. in file TestGDBRemotePlatformFile.py
To fix this, use the
platform put-file
method, which is used in theredirect_stdin
from this test already.