Skip to content

test: add test cases for main#1388

Merged
amilcarlucas merged 2 commits intoArduPilot:masterfrom
OmkarSarkar204:bdd-main-coverage
Mar 21, 2026
Merged

test: add test cases for main#1388
amilcarlucas merged 2 commits intoArduPilot:masterfrom
OmkarSarkar204:bdd-main-coverage

Conversation

@OmkarSarkar204
Copy link
Contributor

@OmkarSarkar204 OmkarSarkar204 commented Mar 18, 2026

Fixes: #1352

Tests addded for __main__.py:

  • register_plugins
  • validate_plugin_registry
  • display_first_use_documentation
  • connect_to_fc_and_set_vehicle_type
  • resolve_writable_vehicle_dir_for_initial_download
  • initialize_filesystem
  • vehicle_directory_selection
  • component_editor
  • process_component_editor_results
  • backup_fc_parameters
  • parameter_editor_and_uploader

@OmkarSarkar204 OmkarSarkar204 marked this pull request as draft March 18, 2026 12:37
@OmkarSarkar204 OmkarSarkar204 changed the title test: add few test cases for main test: add test cases for main Mar 18, 2026
@OmkarSarkar204
Copy link
Contributor Author

Im planning to add a few more tests, will remove it from draft when ready.

@coveralls
Copy link

coveralls commented Mar 19, 2026

Pull Request Test Coverage Report for Build 23251116424

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 93.188%

Totals Coverage Status
Change from base Build 23238548626: 0.1%
Covered Lines: 10944
Relevant Lines: 11744

💛 - Coveralls

@OmkarSarkar204 OmkarSarkar204 marked this pull request as ready for review March 20, 2026 05:46
@OmkarSarkar204
Copy link
Contributor Author

OmkarSarkar204 commented Mar 20, 2026

Hello @amilcarlucas , can you please take a look at this and check if its ok?

@amilcarlucas amilcarlucas added the enhancement New feature or request label Mar 20, 2026
Copy link
Collaborator

@amilcarlucas amilcarlucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Looks good at first sight. I rebased it to re-trigger the tests.

@amilcarlucas amilcarlucas requested a review from Copilot March 20, 2026 11:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds unit tests covering key control-flow branches in ardupilot_methodic_configurator.__main__ to address #1352 and improve confidence in startup orchestration, plugin validation, filesystem setup, editor flows, backups, and parameter upload behavior.

Changes:

  • Added targeted tests for plugin registration/validation and first-use documentation URL behavior
  • Added tests for connection error handling, writable-dir fallback, and filesystem initialization fatal-paths
  • Added tests for component/parameter editor paths, backups, GPS param rename behavior, and main() orchestration

# Assert
assert result == fallback
mock_warn.assert_called_once()
warn_kwargs: dict = mock_warn.call_args[0][1]
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mock_warn.call_args[0][1] assumes logging_warning is called with at least two positional args and that the second is a dict. If logging_warning uses keyword args (common for structured logging) or only one positional arg, this will raise (IndexError/TypeError) and make the test incorrectly fail. Prefer asserting against mock_warn.call_args.kwargs (or mock_warn.call_args[1]) and only fall back to positional arg inspection if the production signature truly passes a dict positionally.

Suggested change
warn_kwargs: dict = mock_warn.call_args[0][1]
args, kwargs = mock_warn.call_args
if kwargs:
warn_kwargs: dict = kwargs
elif len(args) > 1 and isinstance(args[1], dict):
warn_kwargs = args[1]
else:
pytest.fail("logging_warning was not called with expected keyword or dict positional arguments")

Copilot uses AI. Check for mistakes.
Comment on lines +1954 to +1957
patch(
"ardupilot_methodic_configurator.__main__.ProgramSettings.get_setting",
return_value=False,
),
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patching ProgramSettings.get_setting with a blanket return_value=False makes the test brittle and can accidentally push main() down unintended branches if main() (now or later) reads a non-boolean setting (e.g., string-valued settings like GUI mode). Using a side_effect that returns specific values per key (or patching only the specific settings read in this test) will keep the test focused and reduce false failures when main() evolves.

Copilot uses AI. Check for mistakes.
validate_plugin_registry(mock_fs)

# Assert
mock_err.assert_called_once()
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion only verifies that an error was logged, but not that it identifies the missing plugin. To make the test protect the diagnostic behavior (and prevent regressions where the wrong plugin name is reported), assert that the logged call includes "unknown_plugin" (either in args or kwargs, depending on logging_error's signature).

Suggested change
mock_err.assert_called_once()
mock_err.assert_called_once()
args, kwargs = mock_err.call_args
# Ensure that the logged error message identifies the missing plugin
combined = " ".join([str(a) for a in args] + [str(v) for v in kwargs.values()])
assert "unknown_plugin" in combined

Copilot uses AI. Check for mistakes.
Adds BDD-style unit tests covering key control-flow branches and initial
setup orchestration in `ardupilot_methodic_configurator.__main__`.

Specific test additions include:
- Validation logic for plugin registration (motor_test, battery_monitor).
- Failure pathways for connection timeouts and non-writable directory fallbacks.
- Correct firmware documentation rendering based on user application state.
- Parameter editor pathing, component editor flows, and legacy GPS param renames.
- Proper logging propagation on disk space exhaustion during FC backups.
- Mocked end-to-end `main()` execution confirming graceful exit loops.

Fixes: ArduPilot#1352

Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
Signed-off-by: Omkar Sarkar <omkarsarkar24@gmail.com>
@OmkarSarkar204
Copy link
Contributor Author

Fixed the pylint error , should be passing now :)

Copy link
Collaborator

@amilcarlucas amilcarlucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the tests, and the pylint fix. merged

@amilcarlucas amilcarlucas merged commit 9383df7 into ArduPilot:master Mar 21, 2026
18 of 19 checks passed
@OmkarSarkar204 OmkarSarkar204 deleted the bdd-main-coverage branch March 21, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Increase test coverage using BDD tests

4 participants