Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix spellcheck and missing timeout in sshdriver #286

Merged
merged 6 commits into from Aug 13, 2018
Merged

Fix spellcheck and missing timeout in sshdriver #286

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7418d31
driver/sshdriver: fix errors introduced with commit 1f89ed9f
kjeldflarup Jul 10, 2018
a50f6af
driver/sshdriver: adjust remotepath kwarg in put()
Bastian-Krause Jul 20, 2018
bb31a44
commandmixin.run_check was missing codec parameters
kjeldflarup Jul 13, 2018
0e5d178
driver/bareboxdriver: use _run() method signature everywhere
Bastian-Krause Jul 26, 2018
9228feb
driver/smallubootdriver.py: use _run() method signature everywhere an…
Bastian-Krause Jul 26, 2018
944a4d7
driver/ubootdriver.py: use _run() method signature everywhere and hon…
Bastian-Krause Jul 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions labgrid/driver/bareboxdriver.py
View file
Open in desktop
Expand Up @@ -64,7 +64,7 @@ def on_deactivate(self):
def run(self, cmd: str, *, timeout: int = 30): # pylint: disable=unused-argument
return self._run(cmd, timeout=timeout)

def _run(self, cmd: str, *, timeout: int = 30): # pylint: disable=unused-argument
def _run(self, cmd: str, *, timeout: int = 30, codec: str = "utf-8", decodeerrors: str = "strict"): # pylint: disable=unused-argument,line-too-long
"""
Runs the specified command on the shell and returns the output.

Expand All @@ -75,7 +75,7 @@ def _run(self, cmd: str, *, timeout: int = 30): # pylint: disable=unused-argume
Returns:
Tuple[List[str],List[str], int]: if successful, None otherwise
"""
# FIXME: Handle pexpect Timeout
# FIXME: use codec, decodeerrors
marker = gen_marker()
# hide marker from expect
hidden_marker = '"{}""{}"'.format(marker[:4], marker[4:])
Expand Down
8 changes: 4 additions & 4 deletions labgrid/driver/commandmixin.py
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ def wait_for(self, cmd, pattern, timeout=30.0, sleepduration=1):
if timeout.expired:
raise ExecutionError("Wait timeout expired")

def _run_check(self, cmd: str, timeout=30):
def _run_check(self, cmd: str, *, timeout=30, codec: str = "utf-8", decodeerrors: str = "strict"):
"""
Internal function which runs the specified command on the shell and
returns the output if successful, raises ExecutionError otherwise.
Expand All @@ -34,14 +34,14 @@ def _run_check(self, cmd: str, timeout=30):
Returns:
List[str]: stdout of the executed command
"""
stdout, stderr, exitcode = self._run(cmd, timeout=timeout)
stdout, stderr, exitcode = self._run(cmd, timeout=timeout, codec=codec, decodeerrors=decodeerrors)
Copy link
Member

Choose a reason for hiding this comment

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

Where are the variables codec and decodeerrors supposed to come from? I guess you missed updating
def _run_check(self, cmd: str, timeout=30):
with these arguments, right?

The problem is not every _run() methods knows about codec and decodeerrors, so this breaks everywhere else.

Copy link
Member

Choose a reason for hiding this comment

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

You should either transform the _run() method of BareboxDriver, UBootDriver and SmallUBootDriver or go without CommandMixin's convenience function run_check on targets with decode errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I hope that these run functions will be stable soon and not have more special options.
I will add these but as unused parameters, because I don't see any use for codec's in these drivers.
Better test functions would help a lot. In my PR for the remote_tmpdir I actually have a test case which covers the SSH driver better.

if exitcode != 0:
raise ExecutionError(cmd, stdout, stderr)
return stdout

@Driver.check_active
@step(args=['cmd'], result=True)
def run_check(self, cmd: str, timeout=30):
def run_check(self, cmd: str, *, timeout=30, codec="utf-8", decodeerrors="strict"):
"""
External run_check function, only available if the driver is active.
Runs the supplied command and returns the stdout, raises an
Expand All @@ -53,4 +53,4 @@ def run_check(self, cmd: str, timeout=30):
Returns:
List[str]: stdout of the executed command
"""
return self._run_check(cmd, timeout)
return self._run_check(cmd, timeout=timeout, codec=codec, decodeerrors=decodeerrors)
5 changes: 3 additions & 2 deletions labgrid/driver/smallubootdriver.py
View file
Open in desktop
Expand Up @@ -58,14 +58,15 @@ def _await_prompt(self):
# wait until UBoot has reached it's prompt
self.console.expect(self.prompt)

def _run(self, cmd):
def _run(self, cmd: str, *, timeout: int = 30, codec: str = "utf-8", decodeerrors: str = "strict"): # pylint: disable=unused-argument,line-too-long
"""
If Uboot is in Command-Line mode: Run command cmd and return it's
output.

Arguments:
cmd - Command to run
"""
# TODO: use codec, decodeerrors

# Check if Uboot is in command line mode
if self._status != 1:
Expand All @@ -85,7 +86,7 @@ def _run(self, cmd):
)

self.console.sendline(cmp_command)
_, before, _, _ = self.console.expect(self.prompt)
_, before, _, _ = self.console.expect(self.prompt, timeout=timeout)

data = self.re_vt100.sub(
'', before.decode('utf-8'), count=1000000
Expand Down
6 changes: 3 additions & 3 deletions labgrid/driver/sshdriver.py
View file
Open in desktop
Expand Up @@ -101,9 +101,9 @@ def _check_master(self):
@Driver.check_active
@step(args=['cmd'], result=True)
def run(self, cmd, codec="utf-8", decodeerrors="strict", timeout=None): # pylint: disable=unused-argument
return self._run(cmd, codec=codec, decodererrors=decodeerrors)
return self._run(cmd, codec=codec, decodeerrors=decodeerrors)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we pass on the timeout value to _run() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Timeout is actually ignored. commandmixin just happen to pass it to sshdriver.
The new is that commandmixin now calls _run instead of run, so actually it could be removed here, but that would not be backwards compatible.


def _run(self, cmd, codec, decodeerrors): # pylint: disable=unused-argument
def _run(self, cmd, codec="utf-8", decodeerrors="strict", timeout=None): # pylint: disable=unused-argument
Copy link
Contributor

Choose a reason for hiding this comment

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

If timeout value is passed on from run() to _run(), I don't see why any default values are needed here.

"""Execute `cmd` on the target.

This method runs the specified `cmd` as a command on its target.
Expand Down Expand Up @@ -143,7 +143,7 @@ def get_status(self):

@Driver.check_active
@step(args=['filename', 'remotepath'])
def put(self, filename, remotepath=None):
def put(self, filename, remotepath=''):
transfer_cmd = "scp {prefix} -P {port} {filename} {user}@{host}:{remotepath}".format(
filename=filename,
user=self.networkservice.username,
Expand Down
6 changes: 3 additions & 3 deletions labgrid/driver/ubootdriver.py
View file
Open in desktop
Expand Up @@ -64,8 +64,8 @@ def on_deactivate(self):
"""
self._status = 0

def _run(self, cmd):
# FIXME: Handle pexpect Timeout
def _run(self, cmd: str, *, timeout: int = 30, codec: str = "utf-8", decodeerrors: str = "strict"): # pylint: disable=unused-argument,line-too-long
# TODO: use codec, decodeerrors
# TODO: Shell Escaping for the U-Boot Shell
marker = gen_marker()
cmp_command = """echo '{}''{}'; {}; echo "$?"; echo '{}''{}';""".format(
Expand All @@ -77,7 +77,7 @@ def _run(self, cmd):
)
if self._status == 1:
self.console.sendline(cmp_command)
_, before, _, _ = self.console.expect(self.prompt)
_, before, _, _ = self.console.expect(self.prompt, timeout=timeout)
# Remove VT100 Codes and split by newline
data = self.re_vt100.sub(
'', before.decode('utf-8'), count=1000000
Expand Down