Skip to content

Add BDD tests for about popup, bin log selection, derived params, and connection renaming#1639

Open
amilcarlucas wants to merge 2 commits into
masterfrom
more_tests
Open

Add BDD tests for about popup, bin log selection, derived params, and connection renaming#1639
amilcarlucas wants to merge 2 commits into
masterfrom
more_tests

Conversation

@amilcarlucas
Copy link
Copy Markdown
Collaborator

Description

  • 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

Checklist

  • Run pre-commit checks locally
  • Verified by a human programmer
  • All commits are signed off (use git commit --signoff)
  • Code follows our coding standards
  • Documentation updated if needed
  • No breaking changes or properly documented

Testing

Describe how you tested these changes:

  • Unit tests pass
  • Integration tests pass
  • Manual testing performed
  • Tested on flight controller hardware

Copilot AI review requested due to automatic review settings May 25, 2026 14:11
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

assert isinstance(call_args, str), f"Expected URL string but got {type(call_args)}"
call_args_lower = call_args.lower()
assert "issues" in call_args_lower or "bug" in call_args_lower, f"Issues URL not found in {call_args}"
assert "github.com" in call_args_lower, f"GitHub URL not found in {call_args}"
@github-actions
Copy link
Copy Markdown
Contributor

Test Results

     4 files  ±  0       4 suites  ±0   40m 31s ⏱️ -22s
 3 790 tests + 33   3 788 ✅ + 33   2 💤 ±0  0 ❌ ±0 
14 952 runs  +132  14 927 ✅ +130  25 💤 +2  0 ❌ ±0 

Results for commit b884c67. ± Comparison against base commit dd1e68a.

@github-actions
Copy link
Copy Markdown
Contributor

☂️ Code Coverage

current status: ✅

Overall Coverage

Statements Covered Coverage Threshold Status
12706 12012 95% 89% 🟢

New Files

No new covered files...

Modified Files

No covered modified files...

updated for commit: b884c67 by action🐍

@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 26404779401

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.1%) to 96.823%

Details

  • Coverage increased (+0.1%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 14132
Covered Lines: 13683
Line Coverage: 96.82%
Relevant Branches: 2533
Covered Branches: 2438
Branch Coverage: 96.25%
Branches in Coverage %: No
Coverage Strength: 3.35 hits per line

💛 - Coveralls

Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comment on lines +26 to +34
@pytest.fixture
def root() -> Generator[tk.Tk, None, None]:
"""Provide a hidden Tk root window for tests."""
root = tk.Tk()
root.withdraw()
yield root
root.update_idletasks()
root.destroy()

"ardupilot_methodic_configurator.frontend_tkinter_base_window.BaseWindow._get_dpi_scaling_factor",
return_value=1.0,
),
patch("ardupilot_methodic_configurator.frontend_tkinter_base_window.BaseWindow.center_window"),
Comment on lines +212 to +218
def setUp(self) -> None:
self.root = tk.Tk()
self.root.withdraw()

def tearDown(self) -> None:
self.root.update_idletasks()
self.root.destroy()
…ms, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants