Motor test tests#1055
Merged
Merged
Conversation
Refactor the code for tesability
Tests for the configuration-step processor were added/updated as part of the same change.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage for the motor test functionality, bringing both the frontend (Tkinter) and data model test files from omitted to fully tested. The tests follow a behavior-driven development (BDD) approach with extensive use of Given-When-Then documentation patterns. Additionally, the PR includes refactoring of the motor test code to improve testability and fix a state leakage bug in the configuration step processor.
Key Changes
- New comprehensive test suite for
frontend_tkinter_motor_test.pycovering UI interactions, status callbacks, and error handling with 907 lines of BDD-style tests - Extensive test expansion for
data_model_motor_test.pyadding ~1200 lines of tests covering settings validation, frame selection workflows, battery feedback, parameter guards, and motor execution workflows - Code refactoring to extract reusable methods (
run_single_motor_test,run_all_motors_test, etc.) and introduceMotorStatusEventenum for better separation of concerns - Bug fix in configuration step processor preventing helper variable leakage between steps
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_frontend_tkinter_motor_test.py |
New comprehensive test suite (907 lines) covering view initialization, widget interactions, motor test workflows, keyboard shortcuts, error handling, and CLI argument parsing |
tests/test_data_model_motor_test.py |
Added 1200+ lines of tests for settings validation, frame selection, battery status, parameter guards, and motor execution workflows with extensive edge case coverage |
tests/test_data_model_configuration_step.py |
Added test to verify connection renaming state doesn't leak between configuration steps |
ardupilot_methodic_configurator/frontend_tkinter_motor_test.py |
Refactored motor test methods to use new model API (run_*_test methods), extracted _ensure_first_test_confirmation and _handle_status_event helpers, simplified motor grid update logic |
ardupilot_methodic_configurator/data_model_motor_test.py |
Added MotorStatusEvent enum and callback type, introduced run_*_test convenience methods, added set_motor_spin_arm_value/set_motor_spin_min_value with validation, implemented first-test warning acknowledgement tracking, added update_frame_type_by_key method |
ardupilot_methodic_configurator/data_model_configuration_step.py |
Fixed helper variable leakage by using variables.copy() and explicitly removing new_connection_prefix when not used in current step |
.coveragerc |
Removed frontend_tkinter_motor_test.py from omit list to track coverage now that comprehensive tests exist |
Comments suppressed due to low confidence (2)
tests/test_frontend_tkinter_motor_test.py:18
- Import of 'Any' is not used.
from typing import TYPE_CHECKING, Any, Callable, cast # pylint: disable=unused-import
tests/test_frontend_tkinter_motor_test.py:30
- Import of 'MotorTestDataModel' is not used.
from ardupilot_methodic_configurator.data_model_motor_test import ( # pylint: disable=unused-import
MotorTestDataModel,
MotorTestExecutionError,
MotorTestSafetyError,
ParameterError,
ValidationError,
)
Comment on lines
+24
to
+30
| from ardupilot_methodic_configurator.data_model_motor_test import ( # pylint: disable=unused-import | ||
| MotorTestDataModel, | ||
| MotorTestExecutionError, | ||
| MotorTestSafetyError, | ||
| ParameterError, | ||
| ValidationError, | ||
| ) |
There was a problem hiding this comment.
The # pylint: disable=unused-import comments on lines 18 and 24 suggest these imports may not be used in the test file. While some imports might be used only in type annotations (like TYPE_CHECKING context), others like MotorTestDataModel, MotorTestExecutionError, etc. should be actively used in tests. Consider removing genuinely unused imports or using them appropriately.
Suggested change
| from ardupilot_methodic_configurator.data_model_motor_test import ( # pylint: disable=unused-import | |
| MotorTestDataModel, | |
| MotorTestExecutionError, | |
| MotorTestSafetyError, | |
| ParameterError, | |
| ValidationError, | |
| ) |
|
|
||
| import pytest | ||
|
|
||
| import ardupilot_methodic_configurator.frontend_tkinter_motor_test as motor_test_module |
There was a problem hiding this comment.
Module 'ardupilot_methodic_configurator.frontend_tkinter_motor_test' is imported with both 'import' and 'import from'.
Contributor
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
Contributor
Test Results 3 files ± 0 3 suites ±0 30m 48s ⏱️ -13s For more details on these failures and errors, see this check. Results for commit e9a3f75. ± Comparison against base commit 90505e5. This pull request removes 1 and adds 97 tests. Note that renamed tests count towards both. |
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
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.
No description provided.