lf_interop_port_reset_test.py: harden against LANforge API failures#311
Merged
Merged
Conversation
…sient LANforge API failures json_get() returns None on any failed request (bad status, connection error, malformed JSON) with no retry, so a momentary LANforge hiccup during wifi-msgs/port-state lookups could crash the reset loop with an unhandled NoneType error. Add json_get_with_retry(), which retries every 5s for up to 40s before giving up, and route all 9 json_get call sites in InteropPortReset through it. Signed-off-by: Narayana-CT <narayana.pinapatruni@candelatech.com>
…e mgr_ip resolution fail loudly json_get() returns None on any failed request (bad status, connection error, malformed JSON) with no retry, so a momentary LANforge hiccup during wifi-msgs/port-state lookups could crash the reset loop with an unhandled NoneType error. Add json_get_with_retry(), which retries every 5s for up to 40s before giving up, and route all 9 json_get call sites in InteropPortReset through it. Also fold change_port_to_ip() into InteropPortReset as a method (reusing self.name_to_eid/self.json_get_with_retry instead of a throwaway Realm object), and have it abort the test if the manager IP can't be resolved rather than silently continuing with an unresolved port name. Since this now needs an instance to call into, mgr_ip resolution moves to run right after InteropPortReset is constructed instead of before, patching base_interop_profile.server_ip afterward since it captured the unresolved value at construction time. Signed-off-by: Narayana-CT <narayana.pinapatruni@candelatech.com>
… wifi-msgs/adb lookups get_last_wifi_msg, get_time_from_wifi_msgs, and its /adb/ device-list check all indexed straight into the json_get_with_retry() response (a['wifi-messages']['time-stamp'], etc.) with no guard, so a bad or None response would raise TypeError/KeyError and crash whichever reset iteration was in flight. Wrap each in try/except, log the error with the raw response received, and degrade instead of crashing: - get_last_wifi_msg falls back to a 'NA' baseline timestamp. That 'NA' harmlessly propagates into get_time_from_wifi_msgs's own fallback below rather than needing special-case handling. - get_time_from_wifi_msgs's wifi-msgs fetch falls back to marking just that device's row 'NA' for the reset and returns early, so one bad response doesn't take down every other device's stats. - The /adb/ device-list lookup (a secondary android-detection signal used only for clustered LANforge setups) falls back to an empty list, so classification relies on the '1.1.' prefix heuristic alone. This is logged as an error since it can misclassify an android device outside '1.1.' in a clustered setup. Signed-off-by: Narayana-CT <narayana.pinapatruni@candelatech.com> Signed-off-by: Narayana-CT <narayana.pinapatruni@candelatech.com>
… wifi-msgs/adb/port lookups get_last_wifi_msg, get_time_from_wifi_msgs, and its /adb/ and port lookups all indexed straight into the json_get_with_retry() response (a['wifi-messages']['time-stamp'], port_ssid_query['interface']['ssid'], etc.) with no guard, so a bad or None response would raise TypeError/KeyError and crash whichever reset iteration was in flight. Wrap each in try/except, log the error with the raw response received, and degrade instead of crashing: - get_last_wifi_msg falls back to a 'NA' baseline timestamp. That 'NA' harmlessly propagates into get_time_from_wifi_msgs's own fallback below rather than needing special-case handling. - get_time_from_wifi_msgs's wifi-msgs fetch falls back to marking just that device's row 'NA' for the reset and returns early, so one bad response doesn't take down every other device's stats. - The /adb/ device-list lookup (a secondary android-detection signal used only for clustered LANforge setups) falls back to an empty list, so classification relies on the '1.1.' prefix heuristic alone. This is logged as an error since it can misclassify an android device outside '1.1.' in a clustered setup. - The android resource-id + cx-time lookup, and the Windows/Other ssid-ip double-check + cx-time lookups, all fall back to 'NA' for cx time; the ssid-ip double-checks default to "not connected" (0) when the port state can't be verified, since we can't confirm the connection without a readable response. Signed-off-by: Narayana-CT <narayana.pinapatruni@candelatech.com>
…sgs/port fallbacks Two issues surfaced with the 'NA' fallbacks added in 47c63cf: 1. Setting ConnectAttempt/Disconnected/Scanning/Association Rejection/ Connected to the string 'NA' crashes every numeric aggregator that consumes them: create_dict_csv does `0 += stats.get(...) or 0`, and generate_overall_graph/generate_overall_graph_table/ individual_client_info all do raw `+`/`sum()`/`int()` over these fields with no type guard. A single LANforge hiccup on one device's wifi-msgs fetch would crash the very next create_dict_csv() call in the default (non-robot_test) reset loop. Switch that fallback to 0 for the numeric fields instead (matches the existing convention for ambiguous counts elsewhere in this file), and use the Remarks field - which is never summed - to flag the row as "Data unavailable - LANforge API error, stats defaulted to 0" so it isn't misread as a genuinely clean reset. The early return in this fallback also skipped the per-device CSV write at the bottom of get_time_from_wifi_msgs, silently dropping that reset's snapshot if the failing device was the last one processed. Extract that write into write_reset_csvs() and call it from both the fallback and the normal return path. 2. The Windows/Other ssid-ip double-check (used to resolve an ambiguous connected-count) forces connected_count to 0 when the verification call fails. That 0 then flows into the existing Remarks logic a few lines later, which can assert "The Disconnections are seen but Client did not connected to user given SSID." - a confident, specific claim that the device failed to reconnect, when the real cause was an unrelated API error during verification, not a real disconnect. Track the failure with a flag and override Remarks with "Connection state unverified - LANforge API error while double-checking connect count" instead, so a verification failure can no longer masquerade as a real connection failure in the report. Robot-test reporting (generate_report_for_robo and friends) has separate, pre-existing gaps in Remarks visibility and is left untouched for now. VERIFIED CLI: python3 lf_interop_port_reset_test.py --host 192.168.207.75 --mgr_ip 192.168.9.14 --dut DemoDUT --ssid NETGEAR_2G_wpa2 --passwd Password@123 --encryp psk2 --reset 3 --time_int 5 --release 11 Signed-off-by: Narayana-CT <narayana.pinapatruni@candelatech.com>
Sidartha-CT
reviewed
Jul 24, 2026
Comment on lines
+281
to
+288
| a = self.json_get_with_retry("/wifi-msgs/last/1", debug_=True) | ||
| try: | ||
| last = a['wifi-messages']['time-stamp'] | ||
| except (TypeError, KeyError) as e: | ||
| logging.error( | ||
| f"get_last_wifi_msg: LANforge response is not in expected format ({e}). Data received: {a}") | ||
| logging.warning("Could not establish a wifi-msgs baseline timestamp; falling back to 'NA' for this reset.") | ||
| return "NA" |
Contributor
There was a problem hiding this comment.
Previously, if the request received a response, it returned last. If there was no response, it threw an error and exited immediately. Now that you've changed it to return "NA" instead, could you temporarily force it to return "NA" (for example, by commenting out the existing code) and verify the behavior? If you haven't already tested this scenario, could you please check it?
Contributor
Author
There was a problem hiding this comment.
Hi @Sidartha-CT, I have verified the "NA" Scenario. The TestFlow is not disturbing. You can check later commits to see how exactly we are handling the failure scenarios.
Sidartha-CT
approved these changes
Jul 24, 2026
goyalsaurabh06
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
json_get_with_retry, 5s poll for up to 40s) aroundevery LANforge API call in the reset loop, so a transient hiccup no
longer crashes the test with an unhandled NoneType error.
port ssid/ip/cx-time lookups) in try/except: log the raw response on
failure, and either fail loudly where the test genuinely can't proceed
without the data (mgr_ip resolution) or degrade gracefully otherwise.
0(notthe string
'NA', which crashed every downstream aggregator doing raw+/sum()/int()) and flag the row viaRemarksso an API failurecan't be misread as a genuine connection failure in the generated
report.
change_port_to_ipintoInteropPortResetas a method, reusingthe instance's retry-safe
json_get_with_retryinstead of athrowaway
Realmobject.Test plan
python3 lf_interop_port_reset_test.py --host 192.168.207.75 --mgr_ip 192.168.9.14 --dut DemoDUT --ssid NETGEAR_2G_wpa2 --passwd Password@123 --encryp psk2 --reset 3 --time_int 5 --release 11