Final cleanup before v1 (antmicro#50)

This patch consists of:
- code cleanups,
- readme/docs cleanups,
- a simplification of the `command`/`expect` feature,
- a bump of the tag from `v0` to `v1` in the readme.

Signed-off-by: Krzysztof Bieganski <[email protected]>

action.yml

@@ -1,5 +1,5 @@
-name: Renode linux runner action
-description: Test your project inside emulated Linux
+name: Renode Linux Runner Action
+description: Test your project in an emulated Linux system using Renode
 author: Antmicro
 
 inputs:

action/command.py

@@ -29,8 +29,8 @@ class Command:
     Stores a Shell command with custom configuration options
     """
 
-    command: list[str] = field(default_factory=list)
-    expect: list[str] = field(default_factory=list)
+    command: str = field(default_factory=str)
+    expect: str | NoneType = None
     timeout: int | NoneType = -1
     echo: bool | NoneType = None
     check_exit_code: bool | NoneType = None
@@ -53,9 +53,9 @@ def _apply_task_properties(self, keys: list[str], defaults: list[Any], task_valu
     def load_from_dict(dict: Dict[str, Any] | str):
 
         if type(dict) == str:
-            dict = {"command": [dict]}
-        elif type(dict.get("command", None)) == str:
-            dict["command"] = [dict["command"]]
+            dict = {"command": dict}
+        elif not dict.get("command"):
+            dict["command"] = ""
 
         name_map = {name: name for name in dict.keys()} | {
             "check-exit-code": "check_exit_code",
@@ -66,7 +66,7 @@ def load_from_dict(dict: Dict[str, Any] | str):
 
     def apply_vars(self, vars: Dict[str, str]):
         """
-        Resolves variables that were provided with task or are global variables.
+        Resolves variables that were provided with Task or are global variables.
 
         Parameters
         ----------
@@ -75,26 +75,25 @@ def apply_vars(self, vars: Dict[str, str]):
 
         variable_group = r"\$\{\{([\sa-zA-Z0-9_\-]*)\}\}"
 
-        for it, command in enumerate(self.command):
-            for match in re.finditer(variable_group, command):
-                pattern = match[0]
-                var_name = match[1]
+        for match in re.finditer(variable_group, self.command):
+            pattern = match[0]
+            var_name = match[1]
 
-                if var_name in vars:
-                    self.command[it] = self.command[it].replace(pattern, vars[var_name])
-                else:
-                    error(f"Variable {var_name} not found!")
+            if var_name in vars:
+                self.command = self.command.replace(pattern, vars[var_name])
+            else:
+                error(f"Variable {var_name} not found!")
 
 
 @dataclass
 class Task:
     """
-    A task is a block of commands that are performed on one shell and have
+    A Task is a block of commands that are performed on one shell and have
     one basic goal, for example mount the filesystem or install a
     package. It also stores additional parameters like "echo" to print
     shell output on the screen, etc.
 
-    Tasks can depend on other tasks and together form a dependency graph.
+    Tasks can depend on other Tasks. Together, they form a dependency graph.
     """
 
     name: str
@@ -134,23 +133,24 @@ def load_from_dict(dict: Dict[str, Any]) -> 'Task':
         return dacite.from_dict(data_class=Task, data={name_map[name]: value for name, value in dict.items()})
 
     @staticmethod
-    def load_from_yaml(yaml_string: str, config: Dict[str, Any] = {}) -> 'Task':
+    def load_from_yaml(yaml_string: str, overrides: Dict[str, Any] = {}) -> 'Task':
         """
-        Construct the task from yaml.
+        Construct a Task from YAML.
 
         Parameters
         ----------
-        yaml_string : string with yaml
+        yaml_string : string with YAML
+        overrides: additional overrides for Task parameters
         """
 
         obj: Dict[str, Any] = yaml.safe_load(yaml_string)
         if type(obj) is not dict:
             raise yaml.YAMLError
 
-        obj.update(config)
+        obj.update(overrides)
 
         if "name" not in obj.keys():
-            error("task description file must have at least 'name' field")
+            error("Task description file must at least contain a 'name' field")
 
         obj["commands"] = [
             Command.load_from_dict(com)
@@ -160,23 +160,23 @@ def load_from_yaml(yaml_string: str, config: Dict[str, Any] = {}) -> 'Task':
         return Task.load_from_dict(obj)
 
     @staticmethod
-    def form_multiline_string(name: str, string: str, config: Dict[str, Any]) -> 'Task':
+    def from_multiline_string(name: str, string: str, params: Dict[str, Any]) -> 'Task':
         """
-        Construct the task from multiline string of commands.
+        Construct a Task from a multiline string of commands.
 
         Parameters
         ----------
-        name: identifier of the task
+        name: identifier of the Task
         string: multiline string with commands
-        config: additional parameters to Task as dictionary
+        params: Task parameters other than name and commands
         """
 
-        config["name"] = name
-        config["commands"] = [
-            Command(command=[com]) for com in string.splitlines()
+        params["name"] = name
+        params["commands"] = [
+            Command(command=com) for com in string.splitlines()
         ]
 
-        return Task.load_from_dict(config)
+        return Task.load_from_dict(params)
 
     def enable(self, value: bool) -> None:
         self.disabled = not value

action/device/hifive_unleashed/tasks/renode_network.yml

@@ -6,10 +6,10 @@ timeout: 10
 fail-fast: true
 sleep: 0
 commands:
-  - "emulation CreateSwitch \"${{SWITCH_NAME}}\""
-  - "emulation CreateTap \"${{TAP_INTERFACE}}\" \"tap\""
-  - "connector Connect host.tap ${{SWITCH_NAME}}"
-  - "connector Connect sysbus.ethernet ${{SWITCH_NAME}}"
+  - emulation CreateSwitch "${{SWITCH_NAME}}"
+  - emulation CreateTap "${{TAP_INTERFACE}}" "tap"
+  - connector Connect host.tap ${{SWITCH_NAME}}
+  - connector Connect sysbus.ethernet ${{SWITCH_NAME}}
 vars:
   SWITCH_NAME: switch0
   TAP_INTERFACE: tap0

action/device/zynq_7000/tasks/renode_network.yml

@@ -6,10 +6,10 @@ timeout: 10
 fail-fast: true
 sleep: 0
 commands:
-  - "emulation CreateSwitch \"${{SWITCH_NAME}}\""
-  - "emulation CreateTap \"${{TAP_INTERFACE}}\" \"tap\""
-  - "connector Connect host.tap ${{SWITCH_NAME}}"
-  - "connector Connect sysbus.gem0 ${{SWITCH_NAME}}"
+  - emulation CreateSwitch "${{SWITCH_NAME}}"
+  - emulation CreateTap "${{TAP_INTERFACE}}" "tap"
+  - connector Connect host.tap ${{SWITCH_NAME}}
+  - connector Connect sysbus.gem0 ${{SWITCH_NAME}}
 vars:
   SWITCH_NAME: switch0
   TAP_INTERFACE: tap0

action/dispatcher.py

@@ -40,7 +40,7 @@ def __init__(self, board: str, global_vars: Dict[str, str], override_vars: Dict[
         Parameters
         ----------
         global_vars: global variables
-        override_vars: dictionary that stores different dictionaries for each task to override existing variables there
+        override_vars: dictionary that stores a dictionary for each Task that overrides its existing variables
         """
 
         # FilteredStdout is used to remove \r characters from telnet output.
@@ -49,12 +49,12 @@ def __init__(self, board: str, global_vars: Dict[str, str], override_vars: Dict[
 
         init_shells = {
             "host": ["sh", self.default_stdout, [
-                Command(command=[], expect=["#"], timeout=5),
-                Command(command=["screen -d -m renode --disable-xwt"], expect=["#"], timeout=5),
+                Command(command="", expect="#", timeout=5),
+                Command(command="screen -d -m renode --disable-xwt", expect="#", timeout=5),
             ], 5, "#"],
             "renode": ["telnet 127.0.0.1 1234", self.default_stdout, [
-                Command(command=[], expect=["(monitor)"], timeout=5),
-                Command(command=["emulation CreateServerSocketTerminal 3456 \"term\""], expect=["(monitor)"], timeout=5),
+                Command(command="", expect="(monitor)", timeout=5),
+                Command(command="emulation CreateServerSocketTerminal 3456 \"term\"", expect="(monitor)", timeout=5),
             ], 3, r"\([\-a-zA-Z\d\s]+\)"],
             "target": ["telnet 127.0.0.1 3456", self.default_stdout, [], 0, "#"],
         }
@@ -157,7 +157,7 @@ def enable_task(self, name: str, value: bool) -> None:
 
     def delete_task(self, name: str) -> None:
         """
-        Deletes task
+        Deletes a Task
 
         Parameters
         ----------

action/run-in-renode.py

@@ -66,17 +66,17 @@ def configure_board(arch: str, board: str, resc: str, repl: str):
 
 def test_task(test_task_str: str):
 
-    additional_settings = {
+    params = {
         "name": "action_test",
         "shell": "target",
         "requires": ["chroot", "python"],
         "echo": True,
     }
 
     try:
-        return Task.load_from_yaml(test_task_str, config=additional_settings)
+        return Task.load_from_yaml(test_task_str, overrides=params)
     except yaml.YAMLError:
-        return Task.form_multiline_string("action_test", test_task_str, config=additional_settings)
+        return Task.from_multiline_string("action_test", test_task_str, params=params)
 
 
 if __name__ == "__main__":

action/shell.py

@@ -33,25 +33,24 @@ def __init__(self, name: str, spawn_cmd: str, stdout: TextIO, commands: list[Com
         """
         Parameters
         ----------
-        name: shell name
-        spawn_cmd: the starting command that initializes shell
+        name: arbitrary Shell name
+        spawn_cmd: the command that spawns the Shell process
         stdout: if the command is executed in echo mode, the output is redirected to this TextIO
-        commands: adds these initial commands to buffer
+        commands: adds these initial commands to queue
         """
         self.queue: queue.Queue[Command] = queue.Queue()
         self.name: str = name
         self.spawn_cmd: str = spawn_cmd
         self.child: px.spawn = None
         self.default_expect: str = default_expect
-        self.last_option = 0
         self.stdout = stdout
 
         for com in commands:
             self._add_command(com)
 
     def _spawn(self) -> None:
         """
-        Start shell
+        Start the Shell
         """
         retries = 7
 
@@ -75,17 +74,17 @@ def _spawn(self) -> None:
         error(f"Shell {self.name} is not responding")
 
     def _expect(self, command: Command) -> None:
-        self.last_option = self.child.expect(command.expect, timeout=command.timeout)
+        self.child.expect(command.expect, timeout=command.timeout)
 
     def _sendline(self, command: Command) -> None:
         if command.command == []:
             return
 
-        self.child.sendline(command.command[self.last_option])
+        self.child.sendline(command.command)
 
     def _add_command(self, command: Command) -> None:
         """
-        Add command to buffer
+        Add command to queue
 
         Parameters
         ----------
@@ -100,15 +99,15 @@ def add_task(self, task: Task) -> None:
 
             command._apply_task_properties(
                 ["timeout", "expect", "echo", "check_exit_code", "should_fail"],
-                [-1, [], None, None, None],
-                [task.timeout, [self.default_expect], task.echo, task.check_exit_code, task.should_fail]
+                [-1, None, None, None, None],
+                [task.timeout, self.default_expect, task.echo, task.check_exit_code, task.should_fail]
             )
 
             self._add_command(command)
 
     def run_step(self) -> Iterator[int]:
         """
-        Runs single command from buffer per iteration and optionally yields it's error code
+        Runs single command from queue per iteration and yields its error code
         """
 
         def return_code(command: Command):
@@ -120,7 +119,7 @@ def return_code(command: Command):
             self.child.sendline("echo RESULT:${?}")
             self.child.expect(r"RESULT:(\d+)", timeout=10)
             ret = int(self.child.match.group(1))
-            self.child.expect_exact("#", timeout=10)
+            self.child.expect_exact(self.default_expect, timeout=10)
             self.child.logfile_read = self.stdout if command.echo else None
 
             if command.should_fail:
@@ -147,4 +146,4 @@ def return_code(command: Command):
             except px.EOF:
                 error(f"Shell {self.name} is not responding")
             except px.TIMEOUT:
-                error("Timeout!")
+                error(f"Timeout! (shell={self.name}, cmd={''.join(command.command)})")

action/tasks/chroot.yml

@@ -5,14 +5,14 @@ echo: true
 timeout: 5
 fail-fast: true
 commands:
-  - "mount /dev/vda /mnt"
-  - "cd /mnt"
-  - "mount -t proc /proc proc/"
-  - "mount -t sysfs /sys sys/"
-  - "mount -o bind /dev dev/"
-  - "mount -o bind /run run/"
-  - "mount -o bind /tmp tmp/"
-  - "mkdir -p sys/kernel/debug"
-  - "mount -t debugfs nodev sys/kernel/debug"
-  - "chroot /mnt /bin/sh"
-  - "cd /home"
+  - mount /dev/vda /mnt
+  - cd /mnt
+  - mount -t proc /proc proc/
+  - mount -t sysfs /sys sys/
+  - mount -o bind /dev dev/
+  - mount -o bind /run run/
+  - mount -o bind /tmp tmp/
+  - mkdir -p sys/kernel/debug
+  - mount -t debugfs nodev sys/kernel/debug
+  - chroot /mnt /bin/sh
+  - cd /home

action/tasks/devices/gpio.yml

@@ -6,6 +6,6 @@ timeout: 10
 fail-fast: true
 disabled: true
 commands:
-  - "modprobe gpio-mockup gpio_mockup_ranges=${{RANGES}}"
+  - modprobe gpio-mockup gpio_mockup_ranges=${{RANGES}}
 vars:
   RANGES: "0,32"

action/tasks/devices/i2c.yml

@@ -6,6 +6,6 @@ timeout: 10
 fail-fast: true
 disabled: true
 commands:
-  - "modprobe i2c-stub chip_addr=${{CHIP_ADDR}}"
+  - modprobe i2c-stub chip_addr=${{CHIP_ADDR}}
 vars:
   CHIP_ADDR: "0x1C"

action/tasks/devices/vivid.yml

@@ -6,4 +6,4 @@ timeout: 10
 fail-fast: true
 disabled: true
 commands:
-  - "modprobe vivid"
+  - modprobe vivid

action/tasks/host_network.yml

@@ -8,11 +8,11 @@ echo: true
 timeout: 10
 fail-fast: true
 commands:
-  - "ip addr add 172.16.0.1/16 dev ${{TAP_INTERFACE}}"
-  - "ip link set up dev ${{TAP_INTERFACE}}"
-  - "iptables -A FORWARD -i ${{TAP_INTERFACE}} -o ${{HOST_INTERFACE}} -j ACCEPT"
-  - "iptables -A FORWARD -i ${{HOST_INTERFACE}} -o ${{TAP_INTERFACE}} -m state --state RELATED,ESTABLISHED -j ACCEPT"
-  - "iptables -t nat -A POSTROUTING -o ${{HOST_INTERFACE}} -j MASQUERADE"
+  - ip addr add 172.16.0.1/16 dev ${{TAP_INTERFACE}}
+  - ip link set up dev ${{TAP_INTERFACE}}
+  - iptables -A FORWARD -i ${{TAP_INTERFACE}} -o ${{HOST_INTERFACE}} -j ACCEPT
+  - iptables -A FORWARD -i ${{HOST_INTERFACE}} -o ${{TAP_INTERFACE}} -m state --state RELATED,ESTABLISHED -j ACCEPT
+  - iptables -t nat -A POSTROUTING -o ${{HOST_INTERFACE}} -j MASQUERADE
 vars:
   TAP_INTERFACE: tap0
   HOST_INTERFACE: eth0

action/tasks/load_os.yml

@@ -5,10 +5,9 @@ echo: true
 timeout: 5
 fail-fast: true
 commands:
-  - expect:
-    - "buildroot login:"
+  - expect: "buildroot login:"
     timeout: 30
     check-exit-code: False
-  - "root"
-  - "dmesg -n 1"
-  - "date -s \"${{NOW}}\""
+  - root
+  - dmesg -n 1
+  - date -s "${{NOW}}"