Fixing test failures

config/main.py

@@ -858,13 +858,13 @@ def _stop_services():
 
 
 def _get_sonic_services():
-    out = clicommon.run_command("systemctl list-dependencies --plain sonic.target | sed '1d'", return_cmd=True)
+    out, _ = clicommon.run_command("systemctl list-dependencies --plain sonic.target | sed '1d'", return_cmd=True)
     return (unit.strip() for unit in out.splitlines())
 
 
 def _get_delayed_sonic_units(get_timers=False):
-    rc1 = clicommon.run_command("systemctl list-dependencies --plain sonic-delayed.target | sed '1d'", return_cmd=True)
-    rc2 = clicommon.run_command("systemctl is-enabled {}".format(rc1.replace("\n", " ")), return_cmd=True)
+    rc1, _ = clicommon.run_command("systemctl list-dependencies --plain sonic-delayed.target | sed '1d'", return_cmd=True)
+    rc2, _ = clicommon.run_command("systemctl is-enabled {}".format(rc1.replace("\n", " ")), return_cmd=True)
     timer = [line.strip() for line in rc1.splitlines()]
     state = [line.strip() for line in rc2.splitlines()]
     services = []
@@ -899,16 +899,16 @@ def _restart_services():
 
 def _delay_timers_elapsed():
     for timer in _get_delayed_sonic_units(get_timers=True):
-        out = clicommon.run_command("systemctl show {} --property=LastTriggerUSecMonotonic --value".format(timer), return_cmd=True)
+        out, _ = clicommon.run_command("systemctl show {} --property=LastTriggerUSecMonotonic --value".format(timer), return_cmd=True)
         if out.strip() == "0":
             return False
     return True
 
 def _per_namespace_swss_ready(service_name):
-    out = clicommon.run_command("systemctl show {} --property ActiveState --value".format(service_name), return_cmd=True)
+    out, _ = clicommon.run_command("systemctl show {} --property ActiveState --value".format(service_name), return_cmd=True)
     if out.strip() != "active":
         return False
-    out = clicommon.run_command("systemctl show {} --property ActiveEnterTimestampMonotonic --value".format(service_name), return_cmd=True)
+    out, _ = clicommon.run_command("systemctl show {} --property ActiveEnterTimestampMonotonic --value".format(service_name), return_cmd=True)
     swss_up_time = float(out.strip())/1000000
     now =  time.monotonic()
     if (now - swss_up_time > 120):
@@ -933,7 +933,7 @@ def _swss_ready():
     return True
 
 def _is_system_starting():
-    out = clicommon.run_command("sudo systemctl is-system-running", return_cmd=True)
+    out, _ = clicommon.run_command("sudo systemctl is-system-running", return_cmd=True)
     return out.strip() == "starting"
 
 def interface_is_in_vlan(vlan_member_table, interface_name):
@@ -2813,7 +2813,7 @@ def _qos_update_ports(ctx, ports, dry_run, json_data):
         command = "{} {} {} -t {},config-db -t {},config-db -y {} --print-data".format(
             SONIC_CFGGEN_PATH, cmd_ns, from_db, buffer_template_file, qos_template_file, sonic_version_file
         )
-        jsonstr = clicommon.run_command(command, display_cmd=False, return_cmd=True)
+        jsonstr, _ = clicommon.run_command(command, display_cmd=False, return_cmd=True)
 
         jsondict = json.loads(jsonstr)
         port_table = jsondict.get('PORT')

config/vlan.py

@@ -87,7 +87,7 @@ def restart_ndppd():
     ndppd_config_gen_cmd = "sonic-cfggen -d -t /usr/share/sonic/templates/ndppd.conf.j2,/etc/ndppd.conf"
     ndppd_restart_cmd = "supervisorctl restart ndppd"
 
-    output = clicommon.run_command(verify_swss_running_cmd, return_cmd=True)
+    output, _ = clicommon.run_command(verify_swss_running_cmd, return_cmd=True)
 
     if output and output.strip() != "running":
         click.echo(click.style('SWSS container is not running, changes will take effect the next time the SWSS container starts', fg='red'),)

scripts/sonic-bootchart

@@ -50,11 +50,13 @@ def check_bootchart_installed():
 
 def get_enabled_status():
     """ Get systemd-bootchart status """
-    return clicommon.run_command("systemctl is-enabled systemd-bootchart", return_cmd=True)
+    output, _ = clicommon.run_command("systemctl is-enabled systemd-bootchart", return_cmd=True)
+    return output
 
 def get_active_status():
     """ Get systemd-bootchart status """
-    return clicommon.run_command("systemctl is-active systemd-bootchart", return_cmd=True)
+    output, _ = clicommon.run_command("systemctl is-active systemd-bootchart", return_cmd=True)
+    return output
 
 def get_output_files():
     bootchart_output_files = []

show/bgp_quagga_v4.py

@@ -22,7 +22,7 @@ def bgp():
 def summary():
     """Show summarized information of IPv4 BGP state"""
     try:
-        device_output = run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
+        device_output, _ = run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
         get_bgp_summary_extended(device_output)
     except Exception:
         run_command('sudo {} -c "show ip bgp summary"'.format(constants.RVTYSH_COMMAND))

show/bgp_quagga_v6.py

@@ -22,7 +22,7 @@ def bgp():
 def summary():
     """Show summarized information of IPv6 BGP state"""
     try:
-        device_output = run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
+        device_output, _ = run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND), return_cmd=True)
         get_bgp_summary_extended(device_output)
     except Exception:
         run_command('sudo {} -c "show ipv6 bgp summary"'.format(constants.RVTYSH_COMMAND))

show/kdump.py

@@ -49,7 +49,7 @@ def get_kdump_oper_mode():
                   returns "Not Ready";
     """
     oper_mode = "Not Ready"
-    command_stdout = clicommon.run_command("/usr/sbin/kdump-config status", return_cmd=True)
+    command_stdout, _ = clicommon.run_command("/usr/sbin/kdump-config status", return_cmd=True)
 
     for line in command_stdout.splitlines():
         if ": ready to kdump" in line:
@@ -99,7 +99,7 @@ def get_kdump_core_files():
     dump_file_list = []
     cmd_message = None
 
-    command_stdout = clicommon.run_command(find_core_dump_files, return_cmd=True)
+    command_stdout, _ = clicommon.run_command(find_core_dump_files, return_cmd=True)
 
     dump_file_list = command_stdout.splitlines()
     if not dump_file_list:
@@ -123,7 +123,7 @@ def get_kdump_dmesg_files():
     dmesg_file_list = []
     cmd_message = None
 
-    command_stdout = clicommon.run_command(find_dmesg_files, return_cmd=True)
+    command_stdout, _ = clicommon.run_command(find_dmesg_files, return_cmd=True)
 
     dmesg_file_list = command_stdout.splitlines()
     if not dmesg_file_list:

show/main.py

@@ -118,7 +118,7 @@ def run_command(command, display_cmd=False, return_cmd=False):
     while True:
         if return_cmd:
             output = proc.communicate()[0]
-            return output
+            return output, proc.returncode
         output = proc.stdout.readline()
         if output == "" and proc.poll() is not None:
             break

tests/config_test.py

@@ -142,13 +142,13 @@ def mock_run_command_side_effect(*args, **kwargs):
 
     if kwargs.get('return_cmd'):
         if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
-            return 'snmp.timer'
+            return 'snmp.timer' , 0
         elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'":
-            return 'swss'
+            return 'swss', 0
         elif command == "systemctl is-enabled snmp.timer":
-            return 'enabled'
+            return 'enabled', 0
         else:
-            return ''
+            return '', 0
 
 def mock_run_command_side_effect_disabled_timer(*args, **kwargs):
     command = args[0]
@@ -158,17 +158,17 @@ def mock_run_command_side_effect_disabled_timer(*args, **kwargs):
 
     if kwargs.get('return_cmd'):
         if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
-            return 'snmp.timer'
+            return 'snmp.timer', 0
         elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'":
-            return 'swss'
+            return 'swss', 0
         elif command == "systemctl is-enabled snmp.timer":
-            return 'masked'
+            return 'masked', 0
         elif command == "systemctl show swss.service --property ActiveState --value":
-            return 'active'
+            return 'active', 0
         elif command == "systemctl show swss.service --property ActiveEnterTimestampMonotonic --value":
-            return '0'
+            return '0', 0
         else:
-            return ''
+            return '', 0
 
 def mock_run_command_side_effect_untriggered_timer(*args, **kwargs):
     command = args[0]
@@ -178,15 +178,15 @@ def mock_run_command_side_effect_untriggered_timer(*args, **kwargs):
 
     if kwargs.get('return_cmd'):
         if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
-            return 'snmp.timer'
+            return 'snmp.timer', 0
         elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'":
-            return 'swss'
+            return 'swss', 0
         elif command == "systemctl is-enabled snmp.timer":
-            return 'enabled'
+            return 'enabled', 0
         elif command == "systemctl show snmp.timer --property=LastTriggerUSecMonotonic --value":
-            return '0'
+            return '0', 0
         else:
-            return ''
+            return '', 0
 
 def mock_run_command_side_effect_gnmi(*args, **kwargs):
     command = args[0]
@@ -196,13 +196,13 @@ def mock_run_command_side_effect_gnmi(*args, **kwargs):
 
     if kwargs.get('return_cmd'):
         if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
-            return 'gnmi.timer'
+            return 'gnmi.timer', 0
         elif command == "systemctl list-dependencies --plain sonic.target | sed '1d'":
-            return 'swss'
+            return 'swss', 0
         elif command == "systemctl is-enabled gnmi.timer":
-            return 'enabled'
+            return 'enabled', 0
         else:
-            return ''
+            return '', 0
 
 
 # Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension.

tests/ip_show_routes_test.py

@@ -30,8 +30,7 @@ def test_show_ip_route_err(
 
         def mock_run_bgp_command(*args, **kwargs):
             command = args[0]
-            print("% Unknown command: show ip route unknown")
-            return 1
+            return "% Unknown command: show ip route unknown", 1
 
         with mock.patch('utilities_common.cli.run_command', mock.MagicMock(side_effect=mock_run_bgp_command)) as mock_run_command:
             runner = CliRunner()

tests/sonic_bootchart_test.py

@@ -51,9 +51,9 @@ def test_disable(self, mock_run_command):
     def test_config_show(self, mock_run_command):
         def run_command_side_effect(command, **kwargs):
             if "is-enabled" in command:
-                return "enabled"
+                return "enabled", 0
             elif "is-active" in command:
-                return "active"
+                return "active", 0
             else:
                 raise Exception("unknown command")
 

utilities_common/bgp_util.py

@@ -1,6 +1,7 @@
 import ipaddress
 import json
 import re
+import sys
 
 import click
 import utilities_common.cli as clicommon
@@ -185,7 +186,10 @@ def run_bgp_command(vtysh_cmd, bgp_namespace=multi_asic.DEFAULT_NAMESPACE, vtysh
     cmd = 'sudo {} {} -c "{}"'.format(
         vtysh_shell_cmd, bgp_instance_id, vtysh_cmd)
     try:
-        output = clicommon.run_command(cmd)
+        output, ret = clicommon.run_command(cmd, return_cmd=True)
+        if ret != 0:
+            click.echo(output.rstrip('\n'))
+            sys.exit(ret)
     except Exception:
         ctx = click.get_current_context()
         ctx.fail("Unable to get summary from bgp {}".format(bgp_instance_id))

utilities_common/cli.py

@@ -539,7 +539,7 @@ def run_command(command, display_cmd=False, ignore_error=False, return_cmd=False
 
     if return_cmd:
         output = proc.communicate()[0]
-        return output
+        return output, proc.returncode
 
     if not interactive_mode:
         (out, err) = proc.communicate()