Add Config.dry_run and more robust error handling in the test loop

This commit adds a `dry_run` method to the config, so that it can be
called before executing tests. The dry run validates the semantic
struture of the operation definitions.

Aditionally this commit adds more robust error handling to the main
test loop.

Author: saulecabrera

test-runner/tests/test_test_case.py

@@ -213,3 +213,77 @@ def test_new_config_with_multiple_proposals(_mock_file: Mock) -> None:
 def test_new_config_should_fail_with_invalid_proposal(_mock_file: Mock) -> None:
     with pytest.raises(ValueError):
         Config.from_file("file")
+
+
+def test_dry_run_valid_config_should_not_raise() -> None:
+    config = Config(operations=[Run(), Wait()])
+    config.dry_run()
+
+
+def test_dry_run_run_without_wait() -> None:
+    config = Config(operations=[Run(), Run()])
+    with pytest.raises(ValueError, match="each Run operation must be paired with a Wait operation"):
+        config.dry_run()
+
+
+def test_dry_run_read_before_run() -> None:
+    config = Config(operations=[Read()])
+    with pytest.raises(ValueError, match="Found Read operation before Run"):
+        config.dry_run()
+
+
+def test_dry_run_wait_before_run() -> None:
+    config = Config(operations=[Wait()])
+    with pytest.raises(ValueError, match="Found Wait operation before Run"):
+        config.dry_run()
+
+
+def test_dry_run_connect_before_run() -> None:
+    config = Config(operations=[Connect()])
+    with pytest.raises(ValueError, match="Found Connect operation before Run"):
+        config.dry_run()
+
+
+def test_dry_run_connect_with_non_tcp_protocol() -> None:
+    config = Config(operations=[Run(), Connect(protocol_type=ProtocolType.UDP), Wait()])
+    with pytest.raises(ValueError, match="udp not supported"):
+        config.dry_run()
+
+
+def test_dry_run_connect_with_duplicate_id() -> None:
+    config = Config(operations=[
+        Run(),
+        Connect(id="conn1"),
+        Connect(id="conn1"),
+        Wait()
+    ])
+    with pytest.raises(ValueError, match="Duplicate definition of id conn1"):
+        config.dry_run()
+
+
+def test_dry_run_send_before_run() -> None:
+    config = Config(operations=[Send(id="conn1", payload="test")])
+    with pytest.raises(ValueError, match="Found Send operation before Run"):
+        config.dry_run()
+
+
+def test_dry_run_send_with_undefined_id() -> None:
+    config = Config(operations=[Run(), Send(id="conn1", payload="test"), Wait()])
+    with pytest.raises(ValueError, match="No identifier defined for conn1"):
+        config.dry_run()
+
+
+def test_dry_run_recv_before_run() -> None:
+    config = Config(operations=[Recv(id="conn1", payload="test")])
+    with pytest.raises(ValueError, match="Found Recv operation before Run"):
+        config.dry_run()
+
+
+def test_dry_run_multiple_errors() -> None:
+    config = Config(operations=[Read(), Wait(), Run(), Run()])
+    with pytest.raises(ValueError) as exc_info:
+        config.dry_run()
+    error_message = str(exc_info.value)
+    assert "Found Read operation before Run" in error_message
+    assert "Found Wait operation before Run" in error_message
+    assert "each Run operation must be paired with a Wait operation" in error_message

test-runner/wasi_test_runner/runtime_adapter.py

@@ -109,7 +109,8 @@ def compute_argv(self, test_path: str,
         return argv
 
     def run_test(self, config: Config, argv: List[str]) -> Result:
-        # pylint: disable=too-many-branches
+        config.dry_run()
+        # pylint: disable=too-many-branches,too-many-locals
         result = Result(is_executed=True, failures=[])
 
         proc: subprocess.Popen[Any] | None = None
@@ -123,36 +124,30 @@ def run_test(self, config: Config, argv: List[str]) -> Result:
                         proc = subprocess.Popen(argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
                         cleanup_dirs = dirs
                     case Read() as read:
-                        if proc is None:
-                            result.failures.append(Failure.unexpected("Read operation called before Run"))
-                        else:
-                            # Instance asserts might seem redudant here, given the match.
-                            # Asserts merely exist to ensure that mypy can fully resolve the underlying type;
-                            # else it will report errors like:
-                            #   wasi_test_runner/runtime_adapter.py:131: error: Argument 2 to "_handle_read"
-                            #   has incompatible type "Read"; expected "Read" [arg-type]
-                            assert isinstance(read, Read)
-                            _handle_read(proc, read, result)
+                        assert proc is not None
+                        # Instance asserts might seem redudant here, given the match.
+                        # Asserts merely exist to ensure that mypy can fully resolve the underlying type;
+                        # else it will report errors like:
+                        #   wasi_test_runner/runtime_adapter.py:131: error: Argument 2 to "_handle_read"
+                        #   has incompatible type "Read"; expected "Read" [arg-type]
+                        assert isinstance(read, Read)
+                        _handle_read(proc, read, result)
                     case Wait() as wait:
-                        if proc is None:
-                            result.failures.append(Failure.unexpected("Wait operation called before Run"))
-                        else:
-                            assert isinstance(wait, Wait)
-                            _handle_wait(proc, wait, result)
+                        assert proc is not None
+                        assert isinstance(wait, Wait)
+                        _handle_wait(proc, wait, result)
+                        # The wait handler will `kill` the process
+                        proc = None
                     case Connect() as conn:
-                        if proc is None:
-                            result.failures.append(Failure.unexpected("Connect operation called before Run"))
-                        else:
-                            assert isinstance(conn, Connect)
-                            _handle_connect(proc, config, conn, result)
+                        assert proc is not None
+                        assert isinstance(conn, Connect)
+                        _handle_connect(proc, config, conn, result)
                     case Send() as send:
                         assert isinstance(send, Send)
                         _handle_send(config, send, result)
                     case Recv() as recv:
                         assert isinstance(recv, Recv)
                         _handle_recv(config, recv, result)
-                    case _:
-                        pass
 
         finally:
             if cleanup_dirs:
@@ -164,44 +159,36 @@ def run_test(self, config: Config, argv: List[str]) -> Result:
                     _, _ = proc.communicate(timeout=5)
                 except subprocess.TimeoutExpired:
                     proc.kill()
-                    result.failures.append(Failure.unexpected("Process timed out"))
+                    out, err = proc.communicate()
+                    msg = "Process timed out"
+                    msg = _append_stdout_and_stderr(msg, out, err)
+                    result.failures.append(Failure.unexpected(msg))
 
         return result
 
 
 def _handle_read(proc: subprocess.Popen[Any], spec: Read, result: Result) -> None:
-    if spec.id == "stdout":
-        if proc.stdout is None:
-            result.failures.append(Failure.unexpected(f"{spec.id} is not available"))
-            return
-        payload = proc.stdout.readline().strip()
-        if payload != spec.payload:
-            result.failures.append(Failure.expectation(f"{spec.id} failed: expected {spec.payload}, got {payload}"))
+    stream = getattr(proc, spec.id, None)
+    if stream is None:
+        result.failures.append(Failure.unexpected(f"{spec} {spec.id} is not available"))
+        return
 
-    if spec.id == "stderr":
-        if proc.stderr is None:
-            result.failures.append(Failure.unexpected(f"{spec.id} is not available"))
-            return
-        payload = proc.stderr.readline().strip()
-        if payload != spec.payload:
-            result.failures.append(Failure.expectation(f"{spec.id} failed: expected {spec.payload}, got {payload}"))
+    payload = stream.readline().strip()
+    if payload != spec.payload:
+        result.failures.append(Failure.expectation(f"{spec} {spec.id} failed: expected {spec.payload}, got {payload}"))
 
 
 def _handle_wait(proc: subprocess.Popen[Any], spec: Wait, result: Result) -> None:
     try:
         out, err = proc.communicate(timeout=5)
         if spec.exit_code != proc.returncode:
             msg = f"{spec} failed: expected {spec.exit_code}, got {proc.returncode}"
-
-            if out:
-                msg += f"\n\n==STDOUT==\n{out}"
-
-            if err:
-                msg += f"\n\n==STDERR==\n{err}"
-
+            msg = _append_stdout_and_stderr(msg, out, err)
             result.failures.append(Failure.expectation(msg))
 
     except subprocess.TimeoutExpired:
+        proc.kill()
+        out, err = proc.communicate()
         result.failures.append(Failure.expectation(f"{spec} failed: timeout expired"))
 
 
@@ -220,35 +207,55 @@ def _handle_connect(proc: subprocess.Popen[Any], config: Config, spec: Connect,
         result.failures.append(Failure.unexpected(f"{spec}: No connection information available"))
         return
 
-    host, port = proc.stdout.readline().strip().split(':')
+    try:
+        line = proc.stdout.readline().strip()
+        parts = line.split(':')
+        if len(parts) != 2:
+            msg = f"{spec}: Expected address information to be available as <host>: <port>, found {line}"
+            result.failures.append(Failure.unexpected(msg))
+            return
+        host, port_str = parts
+        if not host or not port_str:
+            msg = f"{spec}: Expected address information to be available as <host>: <port>, found {line}"
+            result.failures.append(Failure.unexpected(msg))
+            return
+        port = int(port_str)
+    except ValueError as e:
+        result.failures.append(Failure.unexpected(f"{spec}: Failed to parse connection information: {e}"))
+        return
+
+    sock = None
     try:
         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-        sock.connect((host, int(port)))
+        sock.connect((host, port))
         config.connections[spec.id] = sock
-    except socket.timeout:
-        host_port = host + ":" + port
-        result.failures.append(Failure.unexpected(f"{spec}: Could not connect to {host_port}"))
+    except (socket.timeout, ConnectionRefusedError, OSError) as e:
+        if sock is not None:
+            sock.close()
+        result.failures.append(Failure.unexpected(f"{spec}: Could not connect to {host}: {port} - {e}"))
 
 
 def _handle_send(config: Config, spec: Send, result: Result) -> None:
-    if config.connections[spec.id] is None:
-        result.failures.append(Failure.unexpected(f"{spec}: No connection declared for id {spec.id}"))
-        return
-
     sock = config.connections[spec.id]
-    sock.sendall(spec.payload.encode('utf-8'))
+    assert sock is not None
+    try:
+        sock.sendall(spec.payload.encode('utf-8'))
+    except (OSError, socket.error) as e:
+        result.failures.append(Failure.unexpected(f"{spec}: Failed to send data: {e}"))
 
 
 def _handle_recv(config: Config, spec: Recv, result: Result) -> None:
-    if config.connections[spec.id] is None:
-        result.failures.append(Failure.unexpected(f"{spec}: No connection declared for id {spec.id}"))
-        return
-
     sock = config.connections[spec.id]
-    response_bytes = sock.recv(len(spec.payload))
-    response = response_bytes.decode('utf-8')
-    if response != spec.payload:
-        result.failures.append(Failure.unexpected(f"{spec}: Expected {spec.payload}, got {response}"))
+    assert sock is not None
+    try:
+        response_bytes = sock.recv(len(spec.payload))
+        response = response_bytes.decode('utf-8')
+        if response != spec.payload:
+            result.failures.append(Failure.unexpected(f"{spec}: Expected {spec.payload}, got {response}"))
+    except (OSError, socket.error) as e:
+        result.failures.append(Failure.unexpected(f"{spec}: Failed to receive data: {e}"))
+    except UnicodeDecodeError as e:
+        result.failures.append(Failure.unexpected(f"{spec}: Failed to decode response: {e}"))
 
 
 def _cleanup_test_output(dirs: List[Tuple[Path, str]]) -> None:
@@ -258,3 +265,13 @@ def _cleanup_test_output(dirs: List[Tuple[Path, str]]) -> None:
                 f.unlink()
             elif f.is_dir():
                 shutil.rmtree(f)
+
+
+def _append_stdout_and_stderr(msg: str, out: str | None, err: str | None) -> str:
+    if out:
+        msg += f"\n\n==STDOUT==\n{out}"
+
+    if err:
+        msg += f"\n\n==STDERR==\n{err}"
+
+    return msg

test-runner/wasi_test_runner/test_case.py

@@ -239,6 +239,45 @@ def args_env_dirs(self) -> Tuple[List[str], Dict[str, str], List[Tuple[Path, str
     def proposals_as_str(self) -> List[str]:
         return [p.value for p in self.proposals]
 
+    # Performs a dry run of the configuration validating its structure.
+    def dry_run(self) -> None:
+        run_found = False
+        procs: List[str] = []
+        errors: List[str] = []
+
+        for op in self.operations:
+            match op:
+                case Run() as run:
+                    if run_found:
+                        errors.append(f"{run}: each Run operation must be paired with a Wait operation")
+                    run_found = True
+                case Read() as read:
+                    if not run_found:
+                        errors.append(f"{read}: Found Read operation before Run")
+                case Wait() as wait:
+                    if not run_found:
+                        errors.append(f"{wait}: Found Wait operation before Run")
+                    run_found = False
+                case Connect(conn_id, protocol_type) as conn:
+                    if not run_found:
+                        errors.append(f"{conn}: Found Connect operation before Run")
+                    if protocol_type != ProtocolType.TCP:
+                        errors.append(f"{conn}: {protocol_type} not supported")
+                    if conn_id in procs:
+                        errors.append(f"{conn}: Duplicate definition of id {conn_id}")
+                    procs.append(conn_id)
+                case Send(send_id) as send:
+                    if not run_found:
+                        errors.append(f"{send}: Found Send operation before Run")
+                    if send_id not in procs:
+                        errors.append(f"{send}: No identifier defined for {send_id}")
+                case Recv() as recv:
+                    if not run_found:
+                        errors.append(f"{recv}: Found Recv operation before Run")
+
+        if errors:
+            raise ValueError("\n".join(errors))
+
     @classmethod
     def _validate_config(cls: Type[T], dict_config: Dict[str, Any], expected_keys: Set[str]) -> None:
         # Check that the test configuration is unique, either v0 or v1