Create an ssh server to emulate ssh connection #202

pmarkee · 2018-08-29T05:57:00Z

JSRemoteTest-DCO-1.0-Signed-off-by: Peter Marki marpeter@inf.u-szeged.hu

pmarkee · 2018-08-29T05:58:43Z

This patch has the same purpose as #185 , but for ssh devices (currently rpi2 and artik530) instead of serial devices.

rtakacs · 2018-09-20T10:36:06Z

jstest/__init__.py

@@ -14,7 +14,7 @@

 from jstest.builder.builder import Builder
 from jstest.common import console, paths, symbol_resolver, utils
-from jstest.emulate import pseudo_terminal
+from jstest.emulate import pseudo_terminal, mockssh_server


Please fix the order of the modules.

rtakacs · 2018-09-20T10:42:58Z

jstest/__main__.py

@@ -193,12 +203,15 @@ def main():
            testresult.append(env.options.id, env.paths.builddir)
        # Upload all the results to the Firebase database.
        testresult.upload()
+        exitcode = 0


Does the exitcode variable really needed? If the Python script ends normally, the return value is 0. If error happens, it's enough to use the sys.exit(1) to stop the program.

rtakacs · 2018-09-20T10:43:12Z

jstest/__main__.py


    except (Exception, KeyboardInterrupt) as e:
        jstest.resources.patch_modules(env, revert=True)
        jstest.console.error('[%s] %s' % (type(e).__name__, str(e)))

-        sys.exit(1)
+        exitcode = 1


rtakacs · 2018-09-20T10:45:58Z

jstest/emulate/mockssh_server.py

+
+class ThreadPrinter(object):
+    '''
+    #Class to redirect stdout from non-main threads into /dev/null.


This is a multi-line comment, no need # character.

rtakacs · 2018-09-20T10:53:35Z

jstest/emulate/mockssh_server.py

+    #Class to redirect stdout from non-main threads into /dev/null.
+    '''
+    def __init__(self):
+        pass


Unnecessary constructor definition.

Pylint was giving a warning when the constructor was missing. Should I disable that warning in pylintrc or should we keep this constructor?

Nevermind, I just tried it, it's no longer giving a warning. I will delete the constructor.

rtakacs · 2018-09-20T11:14:47Z

.gitignore

@@ -5,3 +5,6 @@
 build/*
 deps/*
 results/*
+
+# Skip ssh key files.
+*.key


Is there any .key file in the projects?

Yes, the mockssh server needs them, so they shouldn't even be in .gitignore, I just realized it.

rtakacs · 2018-09-20T11:43:47Z

jstest/__main__.py

@@ -82,12 +82,19 @@ def parse_options():
                        action='store_true', default=False,
                        help='display less verbose output')

+    parser.add_argument('--emulate',
+                       default=False, action='store_true',
+                       help='Emulate the connection.')


Emulate the connection. -> emulate the connection

rtakacs · 2018-09-20T11:44:59Z

jstest/__main__.py

    group = parser.add_argument_group("Secure Shell communication")

    group.add_argument('--username',
                       metavar='USER',
                       help='specify the username to login to the device.')

+    group.add_argument('--password',
+                       help='specify the password to login to the device.')


There is no need a dot at the end of the text. Could you please modify the help of the --username option as well?

rtakacs · 2018-09-20T12:11:36Z

jstest/emulate/mockssh_server.py

+                break
+
+        self.writeln(ret)
+        self.exit()


Just a shorter solution:

import re iotjs_build_info = re.compile('.*iotjs_build_info.js') if filter(iotjs_build_info.match, self.args): self.writeln('{ "builtins": {}, "features": {}, "stability": "stable" }') self.exit()

rtakacs · 2018-09-24T11:46:41Z

jstest/testrunner/devices/connections/sshcom.py

+            self.ssh.connect(hostname=self.ip, port=self.port, username=self.username,
+                             password=self.password, look_for_keys=False)
+        else:
+            self.ssh.connect(hostname=self.ip, port=self.port, username=self.username)


Just a shorter solution:

look_for_keys=True if self.password: look_for_keys=False self.ssh.connect(hostname=self.ip, port=self.port, username=self.username, password=self.password, look_for_keys=look_for_keys)

Or more simply:

self.ssh.connect(hostname=self.ip, port=self.port, username=self.username, password=self.password, look_for_keys=bool(self.password))

rtakacs · 2018-09-24T11:52:29Z

jstest/testrunner/devices/connections/sshcom.py

        self.ip = device_info['ip']
        self.port = device_info['port']
        self.timeout = device_info['timeout']
+        self._no_exec_command = device_info['_no_exec_command']


It would be nice if this variable could have a comment about its purpose. For example: FIXME: the exec_command of Paramico SSH library can't be used with the emulated SSH server. (or something else).

rtakacs · 2018-09-25T08:03:48Z

jstest/testrunner/devices/connections/sshcom.py

        self.ip = device_info['ip']
        self.port = device_info['port']
        self.timeout = device_info['timeout']
+        # FIXME the exec_command method of paramiko.client.SSHClient cannot be used with the


FIXME -> FIXME:

rtakacs · 2018-09-25T08:16:04Z

jstest/emulate/twisted_server.py

+            exitcode = 1 if 'run_fail' in cmd else 0
+            self.write('{"output": "",'
+                       ' "memstat": {"heap-system": "n/a", "heap-jerry": "n/a", "stack": "n/a"},'
+                       ' "exitcode": %d}\r\n' % exitcode)


exitcode can be replaced with int('fail' in cmd). Please note that JerryScript uses fail folder name for the expected failure tests.

A better solution to format JSON string is to create an object and use json.dumps:

if 'iotjs_build_info' in cmd: result = { 'builtins': {}, 'features': {} 'stability': 'stable' } else: result = { 'output': 'Hello from emulated device.', 'memstat': { 'heap-system': 'n/a', 'heap-jerry': 'n/a', 'stack': 'n/a' }, 'exitcode': int('fail' in cmd) } self.write(json.dumps(result) + '\r\n')

rtakacs · 2018-09-25T08:32:07Z

jstest/__main__.py

+            options.password = 'jerry'
+            options.ip = '127.0.0.1'
+            options.port = 2022
+            options.remote_workdir = 'emulated_workdir'


Unnecessary overwrite.

If we don't overwrite remote_workdir, we will get an error message:
[SystemError] Please use --remote-workdir for the device.
This error message comes from the SSHDevice class, which should not distinguish if we are using an emulated device or a real device, so overwriting remote_workdir is necessary.

JSRemoteTest-DCO-1.0-Signed-off-by: Peter Marki marpeter@inf.u-szeged.hu

rtakacs

LGTM

robertsipka

LGTM

pmarkee changed the title ~~[WIP] Create an ssh server using MockSSH to emulate ssh connection~~ Create an ssh server using MockSSH to emulate ssh connection Sep 20, 2018

rtakacs requested changes Sep 20, 2018

View reviewed changes

pmarkee changed the title ~~Create an ssh server using MockSSH to emulate ssh connection~~ Create an ssh server to emulate ssh connection Sep 24, 2018

rtakacs reviewed Sep 24, 2018

View reviewed changes

rtakacs reviewed Sep 25, 2018

View reviewed changes

Create an ssh server to emulate ssh connection

1ccc26e

JSRemoteTest-DCO-1.0-Signed-off-by: Peter Marki marpeter@inf.u-szeged.hu

rtakacs approved these changes Sep 25, 2018

View reviewed changes

robertsipka approved these changes Sep 25, 2018

View reviewed changes

robertsipka merged commit 2ffd5ba into jerryscript-project:master Sep 25, 2018

pmarkee deleted the mockssh_server branch February 1, 2019 07:41

Create an ssh server to emulate ssh connection #202

Create an ssh server to emulate ssh connection #202

Uh oh!

Conversation

pmarkee commented Aug 29, 2018

Uh oh!

pmarkee commented Aug 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtakacs Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtakacs left a comment

Choose a reason for hiding this comment

Uh oh!

robertsipka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtakacs Sep 20, 2018 •

edited

Loading