ValueError when col and row len do not match + pytest + offending check fixes

Author: monrog2

aci-preupgrade-validation-script.py

@@ -951,13 +951,14 @@ def craftData(column, rows):
         if not (isinstance(rows, list) and isinstance(column, list)):
             raise TypeError("Rows and column must be lists.")
         data = []
-        for i in range(len(rows)):
+        c_len = len(column)
+        for row_entry in range(len(rows)):
+            r_len = len(rows[row_entry])
+            if r_len != c_len:
+                raise ValueError("Row length ({}), data: {} does not match column length ({}).".format(r_len, rows[row_entry], c_len))
             entry = {}
-            for j in range(len(column)):
-                if j < len(rows[i]):
-                    entry[column[j]] = rows[i][j]
-                else:
-                    entry[column[j]] = None
+            for col_pos in range(c_len):
+                entry[column[col_pos]] = rows[row_entry][col_pos]
             data.append(entry)
         return data
 
@@ -1648,7 +1649,7 @@ def switch_group_guideline_check(**kwargs):
 def switch_bootflash_usage_check(tversion, **kwargs):
     result = FAIL_UF
     msg = ''
-    headers = ["Pod-ID", "Node-ID", "Utilization", "Alert"]
+    headers = ["Pod-ID", "Node-ID", "Utilization"]
     data = []
     recommended_action = "Over 50% usage! Contact Cisco TAC for Support"
     doc_url = "https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#switch-node-bootflash-usage"
@@ -2860,7 +2861,7 @@ def apic_version_md5_check(tversion, username, password, **kwargs):
             c.log = LOG_FILE
             c.connect()
         except Exception as e:
-            data.append([apic_name, '-', '-', str(e), '-'])
+            data.append([apic_name, '-', '-', str(e)])
             print_result(node_title, ERROR)
             has_error = True
             continue
@@ -2870,7 +2871,7 @@ def apic_version_md5_check(tversion, username, password, **kwargs):
                   tversion.dot_version)
         except Exception as e:
             data.append([apic_name, '-', '-',
-                         'ls command via ssh failed due to:{}'.format(str(e)), '-'])
+                         'ls command via ssh failed due to:{}'.format(str(e))])
             print_result(node_title, ERROR)
             has_error = True
             continue
@@ -2884,7 +2885,7 @@ def apic_version_md5_check(tversion, username, password, **kwargs):
                   tversion.dot_version)
         except Exception as e:
             data.append([apic_name, str(tversion), '-',
-                         'failed to check md5sum via ssh due to:{}'.format(str(e)), '-'])
+                         'failed to check md5sum via ssh due to:{}'.format(str(e))])
             print_result(node_title, ERROR)
             has_error = True
             continue

tests/test_AciResult.py

@@ -144,7 +144,7 @@
             "critical",
             "failed"
         ),
-        # Check 9: FAIL_O
+        # Check 9: FAIL_O unformatted only
         (
             "fake_func_name_FAIL_O_unformatted_only_test",
             "FAIL_O Unformatted only",
@@ -204,4 +204,19 @@ def test_invalid_headers_or_data(headers, data):
         synth.craftData(
             column=headers,
             rows=data,
-        )
+        )
+
+@pytest.mark.parametrize(
+    "headers, data",
+    [
+        (["col1", "col2"], [["row1"], ["row2"]]),  # Rows are shorter
+        (["col1"], [["row1", "row2"], ["row3", "row4"]]),  # columns are shorter
+    ]
+)
+def test_mismatched_lengths(headers, data):
+    with pytest.raises(ValueError):
+        synth = script.AciResult("func_name", "Check Title", "A Description")
+        synth.craftData(
+            column=headers,
+            rows=data,
+        )

tests/test_run_checks.py

@@ -6,7 +6,7 @@
 script = importlib.import_module("aci-preupgrade-validation-script")
 AciVersion = script.AciVersion
 JSON_DIR = script.JSON_DIR
-ApicResult = script.AciResult
+AciResult = script.AciResult
 Result = script.Result
 check_wrapper = script.check_wrapper
 
@@ -162,10 +162,10 @@ def test_run_checks(capsys, caplog):
                 else:
                     assert data["failureDetails"]["failType"] == ""
                 # failureDetails.data
-                assert data["failureDetails"]["data"] == ApicResult.craftData(
+                assert data["failureDetails"]["data"] == AciResult.craftData(
                     others.get("headers", []), others.get("data", [])
                 )
-                assert data["failureDetails"]["unformatted_data"] == ApicResult.craftData(
+                assert data["failureDetails"]["unformatted_data"] == AciResult.craftData(
                     others.get("unformatted_headers", []), others.get("unformatted_data", [])
                 )
                 # other fields