TODO see inside REWORD ME riotnode: add node implementation and tests

The abstraction is a basic class to replicate behavior that was in
`testrunner.

Implementation was sometime directly taken from `testrunner` without
being technically justified to keep backward compatibility. This is
described in `TODO.rst` and dedicated implementation.

This also adds a test `node` implementation with a Makefile.
It is currently an easy to use node as no output is lost and reset
correctly resets.

TODO maybe put this as a comment in the file!!
When pexpect 'close' a 'ptyprocess' it starts by 'SIGHUP'.
It is not handled by default to call 'atexit'.
To not do a specific handling for 'SIGHUP' just forward all signals to
the child. This wrapper should not do any specific things anyway.

TODO Split the 'safe_term_close' to utils with a test not using Make
term and so directly only wanting 'sigkill'
This should allow merging it before and using it in testrunner

TODO settle the `env` handling.
Get feedback on what to do here.

Disable local echo otherwise pexpect matches sent characters.

REMOVE ME: Include fix for RIOT-OS#10952

dist/pythonlibs/riotnode/TODO.rst

@@ -0,0 +1,24 @@
+TODO list
+=========
+
+Some list of things I would like to do but not for first publication.
+
+
+Legacy handling
+---------------
+
+Some handling was directly taken from ``testrunner``, without a justified/tested
+reason. I just used it to not break existing setup for nothing.
+I have more details in the code.
+
+* Ignoring reset return value and error message
+* Use killpg(SIGKILL) to kill terminal
+
+
+Testing
+-------
+
+The current 'node' implementation is an ideal node where all output is captured
+and reset directly resets. Having wilder implementations with output loss (maybe
+as a deamon with a ``flash`` pre-requisite and sometime no ``reset`` would be
+interesting.

dist/pythonlibs/riotnode/riotnode/node.py

@@ -0,0 +1,174 @@
+"""RIOTNode abstraction.
+
+Define class to abstract a node over the RIOT build system.
+"""
+
+import os
+import time
+import logging
+import subprocess
+import contextlib
+import signal
+
+import pexpect
+
+DEVNULL = open(os.devnull, 'w')
+
+
+class TermSpawn(pexpect.spawn):
+    """Subclass to adapt the behaviour to our need.
+
+    * change default `__init__` values
+      * disable local 'echo' to not match send messages
+      * 'utf-8/replace' by default
+      * default timeout
+    """
+
+    def __init__(self,  # pylint:disable=too-many-arguments
+                 command, timeout=10, echo=False,
+                 encoding='utf-8', codec_errors='replace', **kwargs):
+        super().__init__(command, timeout=timeout, echo=echo,
+                         encoding=encoding, codec_errors=codec_errors,
+                         **kwargs)
+
+
+class RIOTNode():
+    """Class abstracting a RIOTNode in an application.
+
+    This should abstract the build system integration.
+
+    :param application_directory: relative directory to the application.
+    :param env: dictionary of environment variables that should be used.
+                These overwrites values coming from `os.environ` and can help
+                define factories where environment comes from a file or if the
+                script is not executed from the build system context.
+
+    Environment variable configuration
+
+    :environment BOARD: current RIOT board type.
+    :environment RIOT_TERM_START_DELAY: delay before `make term` is said to be
+                                        ready after calling.
+    """
+
+    TERM_SPAWN_CLASS = TermSpawn
+    TERM_STOP_SIGNAL = signal.SIGKILL
+    TERM_STARTED_DELAY = int(os.environ.get('RIOT_TERM_START_DELAY') or 3)
+
+    MAKE_ARGS = ()
+    RESET_TARGETS = ('reset',)
+
+    def __init__(self, application_directory='.', env=None):
+        self._application_directory = application_directory
+
+        # TODO I am not satisfied by this, but would require changing all the
+        # environment handling, just put a note until I can fix it.
+        # I still want to show a PR before this
+        # I would prefer getting either no environment == os.environ or the
+        # full environment to use.
+        # It should also change the `TERM_STARTED_DELAY` thing.
+        self.env = os.environ.copy()
+        self.env.update(env or {})
+
+        self.term = None  # type: pexpect.spawn
+
+        self.logger = logging.getLogger(__name__)
+
+    @property
+    def application_directory(self):
+        """Absolute path to the current directory."""
+        return os.path.abspath(self._application_directory)
+
+    def board(self):
+        """Return board type."""
+        return self.env['BOARD']
+
+    def reset(self):
+        """Reset current node."""
+        # Ignoring 'reset' return value was taken from `testrunner`.
+        # For me it should not be done for all boards as it should be an error.
+        # I would rather fix it in the build system or have a per board
+        # configuration.
+
+        # Make reset yields error on some boards even if successful
+        # Ignore printed errors and returncode
+        self.make_run(self.RESET_TARGETS, stdout=DEVNULL, stderr=DEVNULL)
+
+    @contextlib.contextmanager
+    def run_term(self, **startkwargs):
+        """Terminal context manager."""
+        try:
+            self.start_term(**startkwargs)
+            yield self.term
+        finally:
+            self.stop_term()
+
+    def start_term(self, **spawnkwargs):
+        """Start the terminal.
+
+        The function is blocking until it is ready.
+        It waits some time until the terminal is ready and resets the node.
+        """
+        self.stop_term()
+
+        term_cmd = self.make_command(['term'])
+        self.term = self.TERM_SPAWN_CLASS(term_cmd[0], args=term_cmd[1:],
+                                          env=self.env, **spawnkwargs)
+
+        # on many platforms, the termprog needs a short while to be ready
+        time.sleep(self.TERM_STARTED_DELAY)
+        self.reset()
+
+    def stop_term(self):
+        """Stop the terminal."""
+        self._safe_term_close()
+
+    def _safe_term_close(self):
+        """Safe 'term.close'.
+
+        Handles possible exceptions.
+        """
+        try:
+            self._kill_term()
+        except AttributeError:
+            # Not initialized
+            return
+        except ProcessLookupError:
+            self.logger.warning('Process already stopped')
+
+        self.term.close()
+
+    def _kill_term(self):
+        """Kill the current terminal."""
+        # killpg(SIGKILL) was taken from `testrunner`.
+        # I do not really like direct `SIGKILL` as it prevents script cleanup.
+        # I kept it as I do not want to break an edge case that rely on it.
+
+        # Using 'killpg' shows that our shell script do not correctly kill
+        # programs they started. So this is more a hack than a real solution.
+        os.killpg(os.getpgid(self.term.pid), self.TERM_STOP_SIGNAL)
+
+    def make_run(self, targets, *runargs, **runkwargs):
+        """Call make `targets` for current RIOTNode context.
+
+        It is using `subprocess.run` internally.
+
+        :param targets: make targets
+        :param *runargs: args passed to subprocess.run
+        :param *runkwargs: kwargs passed to subprocess.run
+        :return: subprocess.CompletedProcess object
+            """
+        command = self.make_command(targets)
+        return subprocess.run(command, env=self.env, *runargs, **runkwargs)
+
+    def make_command(self, targets):
+        """Make command for current RIOTNode context.
+
+        :return: list of command arguments (for example for subprocess)
+        """
+        command = ['make']
+        command.extend(self.MAKE_ARGS)
+        if self._application_directory != '.':
+            dir_cmd = '--no-print-directory', '-C', self.application_directory
+            command.extend(dir_cmd)
+        command.extend(targets)
+        return command

dist/pythonlibs/riotnode/riotnode/tests/node_test.py

@@ -0,0 +1,121 @@
+"""riotnode.node test module."""
+
+import os
+import sys
+import tempfile
+
+import pytest
+import pexpect
+
+import riotnode.node
+
+CURDIR = os.path.dirname(__file__)
+APPLICATIONS_DIR = os.path.join(CURDIR, 'utils', 'application')
+
+
+def test_riotnode_application_dir():
+    """Test the creation of a riotnode with an `application_dir`."""
+    riotbase = os.path.abspath(os.environ['RIOTBASE'])
+    application = os.path.join(riotbase, 'examples/hello-world')
+    board = 'native'
+
+    env = {'BOARD': board}
+    node = riotnode.node.RIOTNode(application, env)
+
+    assert node.application_directory == application
+    assert node.board() == board
+
+    clean_cmd = ['make', '--no-print-directory', '-C', application, 'clean']
+    assert node.make_command(['clean']) == clean_cmd
+
+
+def test_riotnode_curdir():
+    """Test the creation of a riotnode with current directory."""
+    riotbase = os.path.abspath(os.environ['RIOTBASE'])
+    application = os.path.join(riotbase, 'examples/hello-world')
+    board = 'native'
+
+    _curdir = os.getcwd()
+    _environ = os.environ.copy()
+    try:
+        os.environ['BOARD'] = board
+        os.chdir(application)
+
+        node = riotnode.node.RIOTNode()
+
+        assert node.application_directory == application
+        assert node.board() == board
+        assert node.make_command(['clean']) == ['make', 'clean']
+    finally:
+        os.chdir(_curdir)
+        os.environ.clear()
+        os.environ.update(_environ)
+
+
+@pytest.fixture(name='app_pidfile_env')
+def fixture_app_pidfile_env():
+    """Environment to use application pidfile"""
+    with tempfile.NamedTemporaryFile() as tmpfile:
+        yield {'PIDFILE': tmpfile.name}
+
+
+def test_running_echo_application(app_pidfile_env):
+    """Test basic functionnalities with the 'echo' application."""
+    env = {'BOARD': 'board', 'APPLICATION': './echo.py'}
+    env.update(app_pidfile_env)
+
+    node = riotnode.node.RIOTNode(APPLICATIONS_DIR, env)
+    node.TERM_STARTED_DELAY = 1
+
+    with node.run_term(logfile=sys.stdout) as child:
+        child.expect_exact('Starting RIOT node')
+
+        # Test multiple echo
+        for i in range(16):
+            child.sendline('Hello Test {}'.format(i))
+            child.expect(r'Hello Test (\d+)', timeout=1)
+            num = int(child.match.group(1))
+            assert i == num
+
+
+def test_running_error_cases(app_pidfile_env):
+    """Test basic functionnalities with the 'echo' application.
+
+    This tests:
+    * stopping already stopped child
+    """
+    # Use only 'echo' as process to exit directly
+    env = {'BOARD': 'board',
+           'NODE_WRAPPER': 'echo', 'APPLICATION': 'Starting RIOT node'}
+    env.update(app_pidfile_env)
+
+    node = riotnode.node.RIOTNode(APPLICATIONS_DIR, env)
+    node.TERM_STARTED_DELAY = 1
+
+    with node.run_term(logfile=sys.stdout) as child:
+        child.expect_exact('Starting RIOT node')
+
+        # Term is already finished and expect should trigger EOF
+        with pytest.raises(pexpect.EOF):
+            child.expect('this should eof')
+
+    # Exiting the context manager should not crash when node is killed
+
+
+def test_expect_not_matching_stdin(app_pidfile_env):
+    """Test that expect does not match stdin."""
+    env = {'BOARD': 'board', 'APPLICATION': './hello.py'}
+    env.update(app_pidfile_env)
+
+    node = riotnode.node.RIOTNode(APPLICATIONS_DIR, env)
+    node.TERM_STARTED_DELAY = 1
+
+    with node.run_term(logfile=sys.stdout) as child:
+        child.expect_exact('Starting RIOT node')
+        child.expect_exact('Hello World')
+
+        msg = "This should not be matched as it is on stdin"
+        child.sendline(msg)
+        matched = child.expect_exact([pexpect.TIMEOUT, msg], timeout=1)
+        assert matched == 0
+        # This would have matched with `node.run_term(echo=True)`

dist/pythonlibs/riotnode/riotnode/tests/utils/application/Makefile

@@ -0,0 +1,16 @@
+.PHONY: all flash reset term
+
+PIDFILE ?= /tmp/riotnode_test_pid
+NODEPID = $(shell cat $(firstword $(wildcard $(PIDFILE)) /dev/null))
+
+NODE_WRAPPER ?= ./node.py
+APPLICATION ?= ./echo.py
+
+all:
+flash:
+
+reset:
+	kill -USR1 $(NODEPID) 2>/dev/null || true
+
+term:
+	sh -c 'echo $$$$ > $(PIDFILE); exec $(NODE_WRAPPER) $(APPLICATION)'

dist/pythonlibs/riotnode/riotnode/tests/utils/application/echo.py

@@ -0,0 +1,16 @@
+#! /usr/bin/env python3
+"""Firmware implementing echoing line inputs."""
+
+import sys
+
+
+def main():
+    """Print some header and echo the output."""
+    print('Starting RIOT node')
+    print('This example will echo')
+    while True:
+        print(input())
+
+
+if __name__ == '__main__':
+    sys.exit(main())

dist/pythonlibs/riotnode/riotnode/tests/utils/application/hello.py

@@ -0,0 +1,17 @@
+#! /usr/bin/env python3
+"""Firmware implementing a simple hello-world."""
+
+import sys
+import signal
+
+
+def main():
+    """Print some header and do nothing."""
+    print('Starting RIOT node')
+    print('Hello World')
+    while True:
+        signal.pause()
+
+
+if __name__ == '__main__':
+    sys.exit(main())