This is an archive of the discontinued LLVM Phabricator instance.

Differential D8023

Fix TestQuoting on remote targets.
ClosedPublic

Authored by chaoren on Mar 2 2015, 6:00 PM.

Details

Reviewers
ovyalov
clayborg
Commits
rGd9c054d0011f: Fix TestQuoting on remote targets.
rLLDB231101: Fix TestQuoting on remote targets.
rL231101: Fix TestQuoting on remote targets.
Summary

Needed to transfer stdout.txt to host before reading.

Diff Detail

Event Timeline

chaoren updated this revision to Diff 21070.Mar 2 2015, 6:00 PM
chaoren retitled this revision from to Fix TestQuoting on remote targets..
chaoren updated this object.
chaoren edited the test plan for this revision. (Show Details)
chaoren added reviewers: ovyalov, clayborg.
chaoren added a subscriber: Unknown Object (MLST).
ovyalov edited edge metadata.Mar 2 2015, 6:14 PM

Please see my comments.

test/settings/quoting/TestQuoting.py
71

No need for semicolon

74

Does it work in case of local run? It seems it might read and write data from/into the same file..

chaoren added inline comments.Mar 2 2015, 7:24 PM
test/settings/quoting/TestQuoting.py
74

I had that concern too, but it works fine on local.

ovyalov added inline comments.Mar 2 2015, 8:36 PM
test/settings/quoting/TestQuoting.py
74

I remember I had such issue with platform.Put when source and destination were in the same directory.
To be safe, please give them different names stdout-(local|remote).txt and check on OSX as well.

chaoren added inline comments.Mar 2 2015, 8:56 PM
test/settings/quoting/TestQuoting.py
74

I guess we could just check if the file already exists, and if it does, assume local, and skip the Get(). Platform cannot tell if we're local, right? Also, platform doesn't provide any functionality to delete remote files, right?

chaoren updated this revision to Diff 21079.Mar 2 2015, 9:01 PM
chaoren edited edge metadata.

Disable on local.

tberghammer added a subscriber: tberghammer.Mar 3 2015, 2:33 AM
tberghammer added inline comments.
test/settings/quoting/TestQuoting.py
71

I think you can check if we run against a remote platform with this condition:

if lldb.remote_platform:
    # Remote
else:
    # Local
74

Platform can remove a file (with the Unlink command) but it is not exposed to the Python interface. If you need it you can expose it easily.

clayborg requested changes to this revision.Mar 3 2015, 9:42 AM
clayborg edited edge metadata.

Please use the "if lldb.remote_platform:" test when doing any remote fetching as tberghammer suggested.

This revision now requires changes to proceed.Mar 3 2015, 9:42 AM
chaoren updated this revision to Diff 21118.Mar 3 2015, 10:29 AM
chaoren edited edge metadata.

Using lldb.remote_platform to check if file transfer is necessary.

clayborg requested changes to this revision.Mar 3 2015, 10:41 AM
clayborg edited edge metadata.

Check my inline comments.

test/settings/quoting/TestQuoting.py
72

Shouldn't this be set to "False" to not resolve the remote copy? Isn't "src_file_spec" the remote path?

73

Isn't this the local copy of the file and so it should be resolved?

This revision now requires changes to proceed.Mar 3 2015, 10:41 AM
chaoren updated this revision to Diff 21123.Mar 3 2015, 10:56 AM
chaoren edited edge metadata.

Accidentally resolved remote file instead of local.

clayborg accepted this revision.Mar 3 2015, 10:58 AM
clayborg edited edge metadata.
This revision is now accepted and ready to land.Mar 3 2015, 10:58 AM
Closed by commit rL231101: Fix TestQuoting on remote targets. (authored by chaoren). · Explain WhyMar 3 2015, 11:13 AM
This revision was automatically updated to reflect the committed changes.
vharron added a subscriber: vharron.Mar 4 2015, 7:00 PM
vharron added inline comments.
lldb/trunk/test/settings/quoting/TestQuoting.py
71 ↗(On Diff #21124)

In other tests, I use the terms "local" and "remote" instead of "src" and "dst"

Revision Contents

PathSize
test/
settings/
quoting/
5 lines

Diff 21123

test/settings/quoting/TestQuoting.py

Show First 20 Lines • Show All 62 Lines • ▼ Show 20 Lines def do_test_args(self, args_in, args_out):
to stdout. Compare the stdout with args_out.""" to stdout. Compare the stdout with args_out."""
self.buildDefault() self.buildDefault()
exe = os.path.join(os.getcwd(), "a.out") exe = os.path.join(os.getcwd(), "a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
self.runCmd("process launch -o stdout.txt -- " + args_in) self.runCmd("process launch -o stdout.txt -- " + args_in)
if lldb.remote_platform:
ovyalovUnsubmitted
Not Done
ReplyInline Actions

No need for semicolon

ovyalov: No need for semicolon
tberghammerUnsubmitted
Not Done
ReplyInline Actions

I think you can check if we run against a remote platform with this condition:

if lldb.remote_platform:
    # Remote
else:
    # Local
tberghammer: I think you can check if we run against a remote platform with this condition: ``` if lldb.
src_file_spec = lldb.SBFileSpec('stdout.txt', False)
clayborgUnsubmitted
Not Done
ReplyInline Actions

Shouldn't this be set to "False" to not resolve the remote copy? Isn't "src_file_spec" the remote path?

clayborg: Shouldn't this be set to "False" to not resolve the remote copy? Isn't "src_file_spec" the…
dst_file_spec = lldb.SBFileSpec('stdout.txt', True)
clayborgUnsubmitted
Not Done
ReplyInline Actions

Isn't this the local copy of the file and so it should be resolved?

clayborg: Isn't this the local copy of the file and so it should be resolved?
lldb.remote_platform.Get(src_file_spec, dst_file_spec);
ovyalovUnsubmitted
Not Done
ReplyInline Actions

Does it work in case of local run? It seems it might read and write data from/into the same file..

ovyalov: Does it work in case of local run? It seems it might read and write data from/into the same…
chaorenAuthorUnsubmitted
Not Done
ReplyInline Actions

I had that concern too, but it works fine on local.

chaoren: I had that concern too, but it works fine on local.
ovyalovUnsubmitted
Not Done
ReplyInline Actions

I remember I had such issue with platform.Put when source and destination were in the same directory.
To be safe, please give them different names stdout-(local|remote).txt and check on OSX as well.

ovyalov: I remember I had such issue with platform.Put when source and destination were in the same…
chaorenAuthorUnsubmitted
Not Done
ReplyInline Actions

I guess we could just check if the file already exists, and if it does, assume local, and skip the Get(). Platform cannot tell if we're local, right? Also, platform doesn't provide any functionality to delete remote files, right?

chaoren: I guess we could just check if the file already exists, and if it does, assume local, and skip…
tberghammerUnsubmitted
Not Done
ReplyInline Actions

Platform can remove a file (with the Unlink command) but it is not exposed to the Python interface. If you need it you can expose it easily.

tberghammer: Platform can remove a file (with the Unlink command) but it is not exposed to the Python…
with open('stdout.txt', 'r') as f: with open('stdout.txt', 'r') as f:
output = f.read() output = f.read()
self.RemoveTempFile("stdout.txt") self.RemoveTempFile("stdout.txt")
self.assertEqual(output, args_out) self.assertEqual(output, args_out)
if __name__ == '__main__': if __name__ == '__main__':
import atexit import atexit
lldb.SBDebugger.Initialize() lldb.SBDebugger.Initialize()
atexit.register(lambda: lldb.SBDebugger.Terminate()) atexit.register(lambda: lldb.SBDebugger.Terminate())
unittest2.main() unittest2.main()