test(data_model_vehicle_components): Added more tests by amilcarlucas · Pull Request #1463 · ArduPilot/MethodicConfigurator

amilcarlucas · 2026-04-04T16:39:58Z

No description provided.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Expands unit-test coverage across the vehicle components data model modules (schema, import, display, validation, templates) and refines a couple of import/validation code paths to better align with expected inputs.

Changes:

Added multiple new unit tests targeting previously uncovered branches across data model modules.
Introduced shared test helper make_fc_schema to simplify minimal schema construction in tests.
Tweaked import/validation logic (ESC protocol lookup default, removed some redundant parsing/guards) and migrated battery voltage tests to pytest style.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/unit_data_model_vehicle_components_json_schema.py	Adds branch-coverage unit tests for `VehicleComponentsJsonSchema` helpers and error logs.
tests/unit_data_model_vehicle_components_import.py	Adds unit tests for battery-type setting and validation-guard behavior.
tests/unit_data_model_vehicle_components_display.py	New unit test file to cover internal `ComponentDataModelDisplay` branches.
tests/unit_data_model_vehicle_components_base.py	Adds unit tests for uncovered branches in `ComponentDataModelBase`.
tests/unit_battery_cell_voltages.py	New pytest-based unit tests for `battery_cell_voltages` functionality.
tests/test_data_model_vehicle_components_validation.py	Adds extensive uncovered-branch tests for validation logic.
tests/test_data_model_vehicle_components_templates.py	Adds test for creating missing `"Components"` root key.
tests/test_data_model_vehicle_components_json_schema.py	Adds a small new branch-coverage test class for schema behavior.
tests/test_data_model_vehicle_components_import.py	Adds many uncovered-branch tests for import edge cases (bitmasks, fallbacks, warnings/errors).
tests/test_data_model_vehicle_components_common.py	Adds `make_fc_schema` helper to support new schema tests.
tests/test_battery_cell_voltages.py	Removes old unittest-based battery voltage tests (replaced by pytest unit tests).
ardupilot_methodic_configurator/data_model_vehicle_components_validation.py	Removes some parsing/ValueError guards and adjusts pylint directives.
ardupilot_methodic_configurator/data_model_vehicle_components_import.py	Fixes ESC protocol lookup default to avoid unintended `"None"` protocol strings.

ardupilot_methodic_configurator/data_model_vehicle_components_validation.py

Copilot · 2026-04-04T16:46:58Z

tests/unit_data_model_vehicle_components_json_schema.py

+        schema = {
+            "properties": {
+                "Components": {
+                    "Flight Controller": {
+                        "description": "FC",
+                        "properties": {"Product": {"description": "Product info"}},
+                    }
+                }
+            }
+        }


This schema shape doesn’t match the other tests (and typical JSON schema structure) where components live under schema["properties"]["Components"]["properties"]. As written, this test may be exercising “malformed schema” behavior rather than the intended “missing section” behavior. Recommendation (mandatory for test correctness): adjust the fixture to nest "Flight Controller" under "Components": {"properties": {...}} so the test isolates the missing-section condition.

tests/unit_battery_cell_voltages.py

Copilot · 2026-04-04T16:46:58Z

tests/unit_battery_cell_voltages.py

+    def test_best_chemistry_for_voltage_returns_none_for_ambiguous_voltage(self) -> None:
+        """best_chemistry_for_voltage returns None when no chemistry gives a near-integer cell count."""
+        # 10.0 V doesn't divide cleanly by common cell voltages → no clear winner
+        result = BatteryCell.best_chemistry_for_voltage(10.0, "Volt per cell max")
+        # May or may not match - the important thing is no exception; result can be None or a string
+        assert result is None or isinstance(result, str)


The docstring states the function “returns None” for ambiguous voltage, but the assertion allows any string as well, which makes the test non-verifying and contradicts its own documentation. Recommendation (mandatory): either (a) assert result is None using a voltage you expect to be ambiguous deterministically, or (b) update the docstring/test intent to reflect the function’s actual contract (e.g., “may return None”) and validate a more specific property of the result.

Suggested change

def test_best_chemistry_for_voltage_returns_none_for_ambiguous_voltage(self) -> None:

"""best_chemistry_for_voltage returns None when no chemistry gives a near-integer cell count."""

# 10.0 V doesn't divide cleanly by common cell voltages → no clear winner

result = BatteryCell.best_chemistry_for_voltage(10.0, "Volt per cell max")

# May or may not match - the important thing is no exception; result can be None or a string

assert result is None or isinstance(result, str)

def test_best_chemistry_for_voltage_returns_none_or_valid_chemistry_for_ambiguous_voltage(self) -> None:

"""best_chemistry_for_voltage may return None or a known chemistry for an ambiguous voltage."""

# 10.0 V may be ambiguous depending on scoring; if a match is returned, it must be a known chemistry.

result = BatteryCell.best_chemistry_for_voltage(10.0, "Volt per cell max")

assert result is None or result in BatteryCell.chemistries()

github-actions · 2026-04-04T17:41:06Z

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
12058	11340	94%	89%	🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: c1dd70f by action🐍

github-actions · 2026-04-04T17:46:51Z

Test Results

4 files ± 0 4 suites ±0 41m 1s ⏱️ +4s
3 518 tests +103 3 516 ✅ +103 2 💤 ±0 0 ❌ ±0
13 864 runs +412 13 840 ✅ +412 24 💤 ±0 0 ❌ ±0

Results for commit f6523d9. ± Comparison against base commit ca776fa.

This pull request removes 9 and adds 112 tests. Note that renamed tests count towards both.

tests.test_battery_cell_voltages.TestBatteryCell ‑ test_chemistries
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_limit_max_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_limit_min_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_recommended_arm_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_recommended_crit_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_recommended_low_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_recommended_max_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_recommended_min_voltage
tests.test_battery_cell_voltages.TestBatteryCell ‑ test_voltage_monoticity

tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_defaults_to_first_element_for_non_i2c_list_battery_type
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_defaults_to_first_i2c_bus_when_batt_i2c_bus_not_provided
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_leaves_esc_protocol_unchanged_when_type_not_in_doc_or_dict
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_logs_error_for_invalid_voltage_type_in_import_bat_voltage
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_logs_error_for_non_integer_mot_pwm_type
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_logs_error_when_gnss_conn_type_is_neither_serial_nor_can
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_logs_error_when_reverse_key_search_length_differs
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_logs_error_when_reverse_key_search_values_not_found
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_logs_warning_when_chemistry_score_is_nan
tests.test_data_model_vehicle_components_import.TestComponentDataModelImportUncoveredBranches ‑ test_system_passes_when_code_and_doc_protocols_match
…

♻️ This comment has been updated with latest results.

Copilot AI review requested due to automatic review settings April 4, 2026 16:40

Copilot AI reviewed Apr 4, 2026

View reviewed changes

Copilot started reviewing on behalf of amilcarlucas April 4, 2026 17:49 View session

amilcarlucas force-pushed the bat_tests branch from fe612c8 to f6523d9 Compare April 6, 2026 21:46

test(data_model_vehicle_components): Added more tests

c1dd70f

amilcarlucas force-pushed the bat_tests branch from f6523d9 to c1dd70f Compare April 6, 2026 22:43

amilcarlucas merged commit 5c97bed into master Apr 6, 2026
23 of 24 checks passed

amilcarlucas deleted the bat_tests branch April 6, 2026 22:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(data_model_vehicle_components): Added more tests#1463

test(data_model_vehicle_components): Added more tests#1463
amilcarlucas merged 1 commit intomasterfrom
bat_tests

amilcarlucas commented Apr 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Uh oh!

github-actions bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Apr 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

amilcarlucas commented Apr 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☂️ Python Coverage

Overall Coverage

New Files

Modified Files

Uh oh!

github-actions bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 4, 2026 •

edited

Loading

github-actions bot commented Apr 4, 2026 •

edited

Loading