Added detailed reason for assert failure for bgp (#19587)

Summary:

Enhanced assertion messages in selected test cases to provide clearer failure context. This improves debuggability and speeds up issue resolution when tests fail.
Added detailed assertion failure messages in test scripts to make it easier to understand why tests fails.

What is the motivation for this PR?
The motivation is to improve test debuggability by adding detailed failure reasons in assertions for selected test cases. This enhancement makes it easier to identify the root cause of test failures in logs, thereby reducing triage time and simplifying test maintenance.

How did you do it?
I updated the assertion statements in the following test files to include descriptive error messages:
tests/bgp/test_bgp_speaker.py
tests/bgp/test_bgp_router_id.py

How did you verify/test it?
Verified that the updated assertion messages are correctly reflected in the test code by reviewing the changes locally.

Author: bachalla

tests/bgp/test_bgp_router_id.py

@@ -23,9 +23,13 @@ def verify_bgp(enum_asic_index, duthost, expected_bgp_router_id, neighbor_type,
     # Verify router id from DUT itself
     pattern = r"BGP router identifier (\d+\.\d+\.\d+\.\d+)"
     match = re.search(pattern, output)
-    pytest_assert(match, "Cannot get actual BGP router id from [{}]".format(output))
-    pytest_assert(match.group(1) == expected_bgp_router_id,
-                  "BGP router id unexpected, expected: {}, actual: {}".format(expected_bgp_router_id, match.group(1)))
+    pytest_assert(match, (
+        "Cannot get actual BGP router id from [{}]. "
+    ).format(output))
+
+    pytest_assert(match.group(1) == expected_bgp_router_id, (
+        "BGP router id unexpected, expected: {}, actual: {}. "
+    ).format(expected_bgp_router_id, match.group(1)))
 
     # Verify BGP sessions are established
     run_bgp_facts(duthost, enum_asic_index)
@@ -40,18 +44,25 @@ def verify_bgp(enum_asic_index, duthost, expected_bgp_router_id, neighbor_type,
             local_ip_map[item["name"]] = item["local_addr"]
 
     for neighbor_name, nbrhost in nbrhosts.items():
-        pytest_assert(neighbor_name in local_ip_map, "Cannot find local ip for {}".format(neighbor_name))
+        pytest_assert(neighbor_name in local_ip_map, (
+            "Cannot find local ip for {}. "
+            "Local IP map: {}"
+        ).format(neighbor_name, local_ip_map))
+
         if neighbor_type == "sonic":
             cmd = "show ip neighbors {}".format(local_ip_map[neighbor_name])
         elif neighbor_type == "eos":
             cmd = "/usr/bin/Cli -c \"show ip bgp neighbors {}\"".format(local_ip_map[neighbor_name])
         output = nbrhost["host"].shell(cmd, module_ignore_errors=True)['stdout']
         pattern = r"BGP version 4, remote router ID (\d+\.\d+\.\d+\.\d+)"
         match = re.search(pattern, output)
-        pytest_assert(match, "Cannot get remote BGP router id from [{}]".format(output))
-        pytest_assert(match.group(1) == expected_bgp_router_id,
-                      "BGP router id is unexpected, local: {}, fetch from remote: {}"
-                      .format(expected_bgp_router_id, match.group(1)))
+        pytest_assert(match, (
+            "Cannot get remote BGP router id from [{}]. "
+        ).format(output))
+
+        pytest_assert(match.group(1) == expected_bgp_router_id, (
+            "BGP router id is unexpected, local: {}, fetch from remote: {}. "
+        ).format(expected_bgp_router_id, match.group(1)))
 
 
 @pytest.fixture()
@@ -129,8 +140,17 @@ def test_bgp_router_id_set(duthosts, enum_frontend_dut_hostname, enum_asic_index
             continue
         output = duthost.shell("show ip bgp neighbor {} advertised-routes| grep {}".format(remote_ip, loopback_ip),
                                module_ignore_errors=True)
-        pytest_assert(output["rc"] == 0, "Failed to check whether Loopback ipv4 address has been advertised")
-        pytest_assert(loopback_ip in output["stdout"], "Router advertised unexpected: {}".format(output["stdout"]))
+        pytest_assert(output["rc"] == 0, (
+            "Failed to check whether Loopback ipv4 address has been advertised. "
+            "Return code: {} "
+            "Output: {}"
+        ).format(output["rc"], output))
+
+        pytest_assert(loopback_ip in output["stdout"], (
+            "Router advertised unexpected. "
+            "Expected loopback IP: {} "
+            "Actual output: {}"
+        ).format(loopback_ip, output["stdout"]))
 
 
 def test_bgp_router_id_set_without_loopback(duthosts, enum_frontend_dut_hostname, enum_asic_index, nbrhosts, request,

tests/bgp/test_bgp_speaker.py

@@ -61,7 +61,9 @@ def change_route(operation, ptfip, neighbor, route, nexthop, port):
     url = "http://%s:%d" % (ptfip, port)
     data = {"command": "neighbor %s %s route %s next-hop %s" % (neighbor, operation, route, nexthop)}
     r = requests.post(url, data=data, proxies={"http": None, "https": None})
-    assert r.status_code == 200
+    assert r.status_code == 200, (
+        "Request failed with status code {}. Expected 200. "
+    ).format(r.status_code)
 
 
 @pytest.fixture(scope="module", autouse=True)
@@ -205,14 +207,27 @@ def test_bgp_speaker_bgp_sessions(common_setup_teardown, duthosts, rand_one_dut_
     """
     duthost = duthosts[rand_one_dut_hostname]
     ptfip, mg_facts, interface_facts, vlan_ips, _, _, speaker_ips, port_num, http_ready = common_setup_teardown
-    assert http_ready
+    assert http_ready, (
+        "HTTP is not ready. "
+        "HTTP Ready: {}"
+    ).format(http_ready)
 
     logger.info("Wait some time to verify that bgp sessions are established")
     time.sleep(20)
     bgp_facts = duthost.bgp_facts()['ansible_facts']
-    assert all([v["state"] == "established" for _, v in list(bgp_facts["bgp_neighbors"].items())]), \
-        "Not all bgp sessions are established"
-    assert str(speaker_ips[2].ip) in bgp_facts["bgp_neighbors"], "No bgp session with PTF"
+    assert all([v["state"] == "established" for _, v in list(bgp_facts["bgp_neighbors"].items())]), (
+        "Not all BGP sessions are established. "
+        "Current state: {} "
+        "BGP neighbors items: {} "
+    ).format(
+        (v["state"] for _, v in list(bgp_facts["bgp_neighbors"].items())),
+        list(bgp_facts["bgp_neighbors"].items())
+    )
+
+    assert str(speaker_ips[2].ip) in bgp_facts["bgp_neighbors"], (
+        "No BGP session with PTF. "
+        "Speaker IP: {} "
+    ).format(str(speaker_ips[2].ip))
 
 
 # For dualtor
@@ -270,7 +285,11 @@ def bgp_speaker_announce_routes_common(common_setup_teardown, tbinfo, duthost,
     """
     ptfip, mg_facts, interface_facts, vlan_ips, _, vlan_if_name, \
         speaker_ips, port_num, http_ready = common_setup_teardown
-    assert http_ready
+    assert http_ready, (
+        "HTTP is not ready. "
+        "HTTP Ready: {}"
+    ).format(http_ready)
+
     asic_type = duthost.facts["asic_type"]
 
     logger.info("announce route")
@@ -283,16 +302,35 @@ def bgp_speaker_announce_routes_common(common_setup_teardown, tbinfo, duthost,
     announce_route(ptfip, lo_addr, peer_range, vlan_ips[0].ip, port_num[2])
 
     logger.info("Wait some time to make sure routes announced to dynamic bgp neighbors")
-    assert wait_until(120, 10, 0, is_all_neighbors_learned, duthost, speaker_ips), \
-        "Not all dynamic neighbors were learned"
+    assert wait_until(120, 10, 0, is_all_neighbors_learned, duthost, speaker_ips), (
+        "Not all dynamic neighbors were learned. "
+        "DUT Host: {}"
+        "Speaker IPs: {}"
+    ).format(duthost, speaker_ips)
 
     logger.info("Verify nexthops and nexthop interfaces for accepted prefixes of the dynamic neighbors")
     rtinfo = duthost.get_ip_route_info(ipaddress.ip_network(prefix.encode().decode()))
     nexthops_ip_set = {str(nexthop.ip) for nexthop in nexthop_ips}
-    assert len(rtinfo["nexthops"]) == 2
+    assert len(rtinfo["nexthops"]) == 2, (
+        "Number of nexthops is not 2. "
+        "Actual number of nexthops: {}"
+        "Nexthops: {}"
+    ).format(len(rtinfo["nexthops"]), rtinfo["nexthops"])
+
     for i in [0, 1]:
-        assert str(rtinfo["nexthops"][i][0]) in nexthops_ip_set
-        assert rtinfo["nexthops"][i][1] == vlan_if_name.encode().decode()
+        assert str(rtinfo["nexthops"][i][0]) in nexthops_ip_set, (
+            "Nexthop IP not found in expected set. "
+            "Actual nexthop IP: {}"
+            "Expected nexthop IP set: {}"
+            "Nexthops: {}"
+        ).format(str(rtinfo["nexthops"][i][0]), nexthops_ip_set, rtinfo["nexthops"])
+
+        assert rtinfo["nexthops"][i][1] == vlan_if_name.encode().decode(), (
+            "Nexthop interface name does not match expected value. "
+            "Actual nexthop interface name: {}"
+            "Expected nexthop interface name: {}"
+            "Nexthops: {}"
+        ).format(rtinfo["nexthops"][i][1], vlan_if_name.encode().decode(), rtinfo["nexthops"])
 
     logger.info("Generate route-port map information")
     extra_vars = {'announce_prefix': prefix,