From cb31c67e548feab12f7b23718ea9c899c0cff687 Mon Sep 17 00:00:00 2001
From: Simon Lin <simonlin@google.com>
Date: Wed, 12 Jan 2022 15:23:14 +0800
Subject: [PATCH] [thread-cert] misc enhancements to improve reliability
 (#7298)

This commit introduces a number of enhancements to `thread-cert`
Border Router tests to improve reliability:
- Checks `ot-rcp` process status for OTBR tests to help catch errors
  earlier
- Make sure `ot-rcp` processes are terminated after one test to avoid
  corrupting subsequent tests
- Upload core dump as artifacts if otbr-agent crashed
---
 .github/workflows/otbr.yml               |  4 ++++
 tests/scripts/thread-cert/node.py        | 25 +++++++++++++++++++++---
 tests/scripts/thread-cert/thread_cert.py |  8 ++++++--
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/otbr.yml b/.github/workflows/otbr.yml
index ecbf2e4d3..bb2c993fe 100644
--- a/.github/workflows/otbr.yml
+++ b/.github/workflows/otbr.yml
@@ -93,6 +93,8 @@ jobs:
           *.pcap
           *.json
           *.log
+          coredump_*
+          otbr-agent_*
     - name: Generate Coverage
       run: |
         ./script/test generate_coverage gcc
@@ -162,6 +164,8 @@ jobs:
           *.pcap
           *.json
           *.log
+          coredump_*
+          otbr-agent_*
     - name: Generate Coverage
       run: |
         ./script/test generate_coverage gcc
diff --git a/tests/scripts/thread-cert/node.py b/tests/scripts/thread-cert/node.py
index 72259b602..ca6087796 100755
--- a/tests/scripts/thread-cert/node.py
+++ b/tests/scripts/thread-cert/node.py
@@ -89,6 +89,14 @@ class OtbrDocker:
                                              stdout=subprocess.DEVNULL,
                                              stderr=subprocess.DEVNULL)
 
+        try:
+            self._ot_rcp_proc.wait(1)
+        except subprocess.TimeoutExpired:
+            # We expect ot-rcp not to quit in 1 second.
+            pass
+        else:
+            raise Exception(f"ot-rcp {nodeid} exited unexpectedly!")
+
     def _get_ot_rcp_path(self) -> str:
         srcdir = os.environ['top_builddir']
         path = '%s/examples/apps/ncp/ot-rcp' % srcdir
@@ -179,14 +187,19 @@ class OtbrDocker:
         self._shutdown_socat()
 
     def _shutdown_docker(self):
-        if self._docker_proc is not None:
+        if self._docker_proc is None:
+            return
+
+        try:
             COVERAGE = int(os.getenv('COVERAGE', '0'))
             OTBR_COVERAGE = int(os.getenv('OTBR_COVERAGE', '0'))
+            test_name = os.getenv('TEST_NAME')
+            unique_node_id = f'{test_name}-{PORT_OFFSET}-{self.nodeid}'
+
             if COVERAGE or OTBR_COVERAGE:
                 self.bash('service otbr-agent stop')
 
-                test_name = os.getenv('TEST_NAME')
-                cov_file_path = f'/tmp/coverage/coverage-{test_name}-{PORT_OFFSET}-{self.nodeid}.info'
+                cov_file_path = f'/tmp/coverage/coverage-{unique_node_id}.info'
                 # Upload OTBR code coverage if OTBR_COVERAGE=1, otherwise OpenThread code coverage.
                 if OTBR_COVERAGE:
                     codecov_cmd = f'lcov --directory . --capture --output-file {cov_file_path}'
@@ -196,6 +209,12 @@ class OtbrDocker:
 
                 self.bash(codecov_cmd)
 
+            copyCore = subprocess.run(f'docker cp {self._docker_name}:/core ./coredump_{unique_node_id}', shell=True)
+            if copyCore.returncode == 0:
+                subprocess.check_call(
+                    f'docker cp {self._docker_name}:/usr/sbin/otbr-agent ./otbr-agent_{unique_node_id}', shell=True)
+
+        finally:
             subprocess.check_call(f"docker rm -f {self._docker_name}", shell=True)
             self._docker_proc.wait()
             del self._docker_proc
diff --git a/tests/scripts/thread-cert/thread_cert.py b/tests/scripts/thread-cert/thread_cert.py
index 514cc4b46..803c0e378 100755
--- a/tests/scripts/thread-cert/thread_cert.py
+++ b/tests/scripts/thread-cert/thread_cert.py
@@ -269,8 +269,12 @@ class TestCase(NcpSupportMixin, unittest.TestCase):
             self._stop_backbone_sniffer()
 
         for node in list(self.nodes.values()):
-            node.stop()
-            node.destroy()
+            try:
+                node.stop()
+            except:
+                traceback.print_exc()
+            finally:
+                node.destroy()
 
         self.simulator.stop()