Skip to content

Commit f2a4efa

Browse files
committed
tests: add BDD tests for about popup, bin log selection, derived params, and connection renaming
- Add TestAboutWindowCreation, TestAboutWindowButtonInteractions, TestAboutWindowUsagePopupPreferences in new test file - Add TestBinLogSelectionWidgets covering success, cancel, and all error paths (VehicleProjectCreationError, VehicleProjectOpenError, OSError) - Add TestDerivedParametersFiltering covering FC-filtered, file-filtered, and offline (no FC) scenarios - Add TestConnectionRenamingWithSameNameSkip covering no-op and conflict cases Quality: - Assert mock_set is called when toggling usage popup checkbox (was vacuous) - _find_button_by_text raises AssertionError with diagnostics if not found - Replace 6-tuple wildcard unpacking with named variables - Assert _duplicates == set() in rename tests - Replace unused bin_log_widget_setup fixture with _make_widget() helper to eliminate duplicated widget construction in every test method
1 parent 0f1c88a commit f2a4efa

4 files changed

Lines changed: 893 additions & 1 deletion

tests/test_data_model_configuration_step.py

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,3 +1006,182 @@ def test_user_receives_expresslrs_warning_with_both_bits_set_and_fltmode_ch_5(se
10061006
assert len(ui_infos) == 1
10071007
assert ui_infos[0][0] == "ExpressLRS Configuration Warning"
10081008
assert ui_errors == []
1009+
1010+
1011+
class TestDerivedParametersFiltering:
1012+
"""Tests for derived parameters filtering logic."""
1013+
1014+
def test_derived_parameters_filtered_by_fc_keys_when_fc_provided(self, processor, mock_local_filesystem) -> None:
1015+
"""
1016+
Derived parameters are filtered by FC keys when fc_parameters is provided.
1017+
1018+
GIVEN: A configuration step with derived parameters and FC parameters
1019+
WHEN: Processing produces derived parameters, some existing in FC and some not
1020+
THEN: Only derived parameters that exist in FC should be collected for application
1021+
AND: Parameters not in FC keys should be excluded from derived_params_to_apply
1022+
"""
1023+
selected_file = "test_file.param"
1024+
# Set up derived parameters that will be returned
1025+
derived_params = {
1026+
"SERIAL1_PROTOCOL": Par(value=5.0, comment="derived"), # in file AND in FC
1027+
"CAN_P1_DRIVER": Par(value=2.0, comment="derived"), # in file, NOT in FC
1028+
}
1029+
mock_local_filesystem.configuration_steps = {selected_file: {"derived": {}}}
1030+
mock_local_filesystem.compute_parameters.return_value = None
1031+
mock_local_filesystem.derived_parameters = {selected_file: derived_params}
1032+
mock_local_filesystem.file_parameters = {
1033+
selected_file: {
1034+
"SERIAL1_PROTOCOL": Par(value=4.0, comment="original"),
1035+
"CAN_P1_DRIVER": Par(value=1.0, comment="original"),
1036+
}
1037+
}
1038+
1039+
# FC only has SERIAL1_PROTOCOL (not CAN_P1_DRIVER)
1040+
limited_fc_params = {"SERIAL1_PROTOCOL": 4.0}
1041+
1042+
_params, ui_errors, _ui_infos, _duplicates, _renames, derived_to_apply = processor.process_configuration_step(
1043+
selected_file, limited_fc_params
1044+
)
1045+
1046+
assert ui_errors == []
1047+
# SERIAL1_PROTOCOL is in both file and FC, so it should be in derived_to_apply
1048+
assert "SERIAL1_PROTOCOL" in derived_to_apply
1049+
assert derived_to_apply["SERIAL1_PROTOCOL"].value == 5.0
1050+
assert derived_to_apply["SERIAL1_PROTOCOL"].comment == "derived"
1051+
# CAN_P1_DRIVER is in file but NOT in FC, so it should be excluded
1052+
assert "CAN_P1_DRIVER" not in derived_to_apply
1053+
# Should have exactly 1 parameter (only the one in FC)
1054+
assert len(derived_to_apply) == 1
1055+
1056+
def test_derived_parameters_not_in_file_are_excluded(self, processor, mock_local_filesystem) -> None:
1057+
"""
1058+
Derived parameters not currently in the file are excluded from derived_params_to_apply.
1059+
1060+
GIVEN: A configuration step with derived parameters where some are add-from-FC shorthands
1061+
WHEN: Processing produces derived parameters
1062+
THEN: Parameters that don't yet exist in the file should be excluded
1063+
AND: Only parameters already in the file should be in derived_params_to_apply
1064+
"""
1065+
selected_file = "test_file.param"
1066+
derived_params = {
1067+
"SERIAL1_PROTOCOL": Par(value=5.0, comment="derived"), # in file
1068+
"NEW_PARAM": Par(value=99.0, comment="derived"), # NOT in file (add-from-FC shorthand)
1069+
}
1070+
mock_local_filesystem.configuration_steps = {selected_file: {"derived": {}}}
1071+
mock_local_filesystem.compute_parameters.return_value = None
1072+
mock_local_filesystem.derived_parameters = {selected_file: derived_params}
1073+
mock_local_filesystem.file_parameters = {
1074+
selected_file: {
1075+
"SERIAL1_PROTOCOL": Par(value=4.0, comment="original"),
1076+
# NEW_PARAM not in file
1077+
}
1078+
}
1079+
1080+
fc_params = {"SERIAL1_PROTOCOL": 4.0, "NEW_PARAM": 99.0}
1081+
1082+
_params, ui_errors, _ui_infos, _duplicates, _renames, derived_to_apply = processor.process_configuration_step(
1083+
selected_file, fc_params
1084+
)
1085+
1086+
assert ui_errors == []
1087+
# Only parameters already in the file should be included
1088+
assert "SERIAL1_PROTOCOL" in derived_to_apply
1089+
assert derived_to_apply["SERIAL1_PROTOCOL"].value == 5.0
1090+
# NEW_PARAM not in file, so should be excluded even though it's in FC and derived_params
1091+
assert "NEW_PARAM" not in derived_to_apply
1092+
# Should have exactly 1 parameter (only the one in file)
1093+
assert len(derived_to_apply) == 1
1094+
1095+
def test_derived_parameters_all_included_when_no_fc_parameters(self, processor, mock_local_filesystem) -> None:
1096+
"""
1097+
All file-matching derived parameters are included when no FC parameters provided.
1098+
1099+
GIVEN: A configuration step with derived parameters but no FC connection
1100+
WHEN: Processing produces derived parameters with empty fc_parameters
1101+
THEN: All derived parameters that exist in the file should be in derived_params_to_apply
1102+
"""
1103+
selected_file = "test_file.param"
1104+
derived_params = {
1105+
"SERIAL1_PROTOCOL": Par(value=5.0, comment="derived"),
1106+
"CAN_P1_DRIVER": Par(value=2.0, comment="derived"),
1107+
}
1108+
mock_local_filesystem.configuration_steps = {selected_file: {"derived": {}}}
1109+
mock_local_filesystem.compute_parameters.return_value = None
1110+
mock_local_filesystem.derived_parameters = {selected_file: derived_params}
1111+
mock_local_filesystem.file_parameters = {
1112+
selected_file: {
1113+
"SERIAL1_PROTOCOL": Par(value=4.0, comment="original"),
1114+
"CAN_P1_DRIVER": Par(value=1.0, comment="original"),
1115+
}
1116+
}
1117+
1118+
# No FC parameters (empty dict simulates offline mode)
1119+
_params, ui_errors, _ui_infos, _duplicates, _renames, derived_to_apply = processor.process_configuration_step(
1120+
selected_file, {}
1121+
)
1122+
1123+
assert ui_errors == []
1124+
# Both should be included since fc_param_keys is empty (no FC filter)
1125+
assert "SERIAL1_PROTOCOL" in derived_to_apply
1126+
assert derived_to_apply["SERIAL1_PROTOCOL"].value == 5.0
1127+
assert "CAN_P1_DRIVER" in derived_to_apply
1128+
assert derived_to_apply["CAN_P1_DRIVER"].value == 2.0
1129+
# Should have both parameters when no FC filtering is applied
1130+
assert len(derived_to_apply) == 2
1131+
1132+
1133+
class TestConnectionRenamingWithSameNameSkip:
1134+
"""Tests for connection renaming edge case where rename results in same name."""
1135+
1136+
def test_rename_operation_skips_when_new_name_equals_old_name(self, processor) -> None:
1137+
"""
1138+
Rename operations are skipped when the new name equals the old name.
1139+
1140+
GIVEN: A parameter that would be renamed to itself (no-op rename)
1141+
WHEN: The rename operation is calculated
1142+
THEN: No rename should be in the result list for that parameter
1143+
AND: No duplicates should be tracked for it
1144+
"""
1145+
# Use a parameter set where CAN1 → CAN1 (same) would be a no-op
1146+
# by using CAN1 as the new_connection_prefix
1147+
parameters = {
1148+
"CAN_P1_DRIVER": Par(value=1.0),
1149+
}
1150+
1151+
# Rename to CAN1 - same connection, which means CAN_P1 → CAN_P1 (no-op)
1152+
_duplicates, renamed_pairs = processor._calculate_connection_rename_operations(parameters, "CAN1", None)
1153+
1154+
# CAN_P1_DRIVER → CAN_P1_DRIVER (no change), should not be in renamed_pairs
1155+
assert ("CAN_P1_DRIVER", "CAN_P1_DRIVER") not in renamed_pairs
1156+
# Should have no renamed pairs since the rename would be a no-op
1157+
assert len(renamed_pairs) == 0
1158+
# Duplicates set is unused by this operation path
1159+
assert _duplicates == set()
1160+
1161+
def test_rename_operation_detects_conflict_with_existing_parameter(self, processor) -> None:
1162+
"""
1163+
Rename operation handles conflict when target name already exists.
1164+
1165+
GIVEN: Parameters where a rename would create a conflict with an existing parameter
1166+
WHEN: The rename operation is calculated
1167+
THEN: The conflicting rename should not be added to renamed_pairs
1168+
AND: No exception should be raised
1169+
"""
1170+
parameters = {
1171+
"CAN_P1_DRIVER": Par(value=1.0),
1172+
"CAN_P2_DRIVER": Par(value=2.0), # Would be the target of CAN_P1 rename to CAN2
1173+
}
1174+
1175+
# Renaming to CAN2 would conflict with existing CAN_P2_DRIVER
1176+
_duplicates, renamed_pairs = processor._calculate_connection_rename_operations(parameters, "CAN2", None)
1177+
1178+
# CAN_P1_DRIVER → CAN_P2_DRIVER but CAN_P2_DRIVER already exists
1179+
# The conflicting rename should be silently skipped
1180+
assert ("CAN_P1_DRIVER", "CAN_P2_DRIVER") not in renamed_pairs
1181+
# Should have no renamed pairs due to conflict
1182+
assert len(renamed_pairs) == 0
1183+
# Duplicates set is unused by this operation path; conflict is tracked via renamed_pairs omission
1184+
assert _duplicates == set()
1185+
# Verify that both parameters are still present (not removed due to conflict)
1186+
assert "CAN_P1_DRIVER" in parameters
1187+
assert "CAN_P2_DRIVER" in parameters

0 commit comments

Comments
 (0)