Skip to content

TST: add branch coverage for fin_flutter_analysis and fix keyword argument bug#904

Merged
Gui-FernandesBR merged 3 commits intoRocketPy-Team:developfrom
erielC:tst/add-fin-flutter-branch-coverage
Dec 4, 2025
Merged

TST: add branch coverage for fin_flutter_analysis and fix keyword argument bug#904
Gui-FernandesBR merged 3 commits intoRocketPy-Team:developfrom
erielC:tst/add-fin-flutter-branch-coverage

Conversation

@erielC
Copy link
Copy Markdown
Collaborator

@erielC erielC commented Dec 4, 2025

Pull request type

  • Code changes (bugfix, features)
  • Code maintenance (refactoring, formatting, tests)

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest tests -m slow --runslow) have passed locally

Current behavior

The fin_flutter_analysis function in rocketpy/utilities.py had several untested code branches:

  • The see_prints=True branch was never executed in tests
  • The see_graphs=True branch was never executed in tests
  • Test coverage for utilities.py was at 68%

Additionally, there was a latent bug where the filename parameter was passed as a positional argument to _flutter_plots(), but the function signature defines it as a keyword-only parameter (after *). This would cause TypeError: _flutter_plots() takes 3 positional arguments but 4 were given when users attempted to save flutter plots to files.

New behavior

Tests Added:

  1. test_fin_flutter_analysis_with_prints - Tests the see_prints=True branch, ensuring print statements execute correctly and the function returns the expected tuple
  2. test_fin_flutter_analysis_with_graphs - Tests the see_graphs=True branch, ensuring plotting functionality works and returns None (uses @patch to mock matplotlib)
  3. test_fin_flutter_analysis_complete_output - Tests both flags enabled simultaneously for complete branch coverage

Bugs Fixed:

  • Fixed keyword argument bug in fin_flutter_analysis (line 286): Changed _flutter_plots(flight, flutter_mach, safety_factor, filename) to _flutter_plots(flight, flutter_mach, safety_factor, filename=filename)
  • Fixed test_flutter_plots to correctly pass filename as a keyword argument

Coverage Impact:

  • Increased utilities.py test coverage from 68% to 69%
  • +2 statements covered (previously red lines now green)

Breaking change

  • No

Additional information

Context: This PR is part of my application for the RocketPy Library Development team (Option 2: Add new tests to improve coverage).

Testing Methodology:

  • All new tests use the existing flight_calisto_custom_wind fixture
  • Tests use @patch("matplotlib.pyplot.show") to mock matplotlib, preventing plot windows during test execution
  • Tests follow existing RocketPy conventions: function-based tests, descriptive docstrings, proper assertions

Quality Checks:

  • Ruff: All checks passed
  • Pylint: 10.00/10 rating
  • Black formatting: Passed
  • All unit tests pass locally

Before Tests:
before_utilities_report

After Tests:
after_utilities_report

This commit increases test coverage for utilities.py by adding three new
tests that cover previously untested branches in the fin_flutter_analysis
function. The tests cover the see_prints=True branch, see_graphs=True
branch, and both flags enabled simultaneously.

During testing, discovered and fixed a bug where the filename parameter
was incorrectly passed as a positional argument to _flutter_plots, which
requires it as a keyword-only argument. This would cause a TypeError at
runtime when users attempted to save flutter plots to files.

Changes:
- Add test_fin_flutter_analysis_with_prints for print branch coverage
- Add test_fin_flutter_analysis_with_graphs for plotting branch coverage
- Add test_fin_flutter_analysis_complete_output for combined coverage
- Fix filename argument bug in fin_flutter_analysis (line 286)
- Fix test_flutter_plots to use keyword argument for filename
- Increases utilities.py coverage from 68% to 69% (+2 statements)
@erielC erielC requested a review from a team as a code owner December 4, 2025 00:59
Copy link
Copy Markdown
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

@erielC congratulations on your first contribution! Code looks clean and good, I appreciate your effort on this, thank you!!

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.01%. Comparing base (9cf3dd4) to head (6dd747d).
⚠️ Report is 21 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #904      +/-   ##
===========================================
+ Coverage    80.27%   81.01%   +0.74%     
===========================================
  Files          104      107       +3     
  Lines        12769    13642     +873     
===========================================
+ Hits         10250    11052     +802     
- Misses        2519     2590      +71     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Gui-FernandesBR
Copy link
Copy Markdown
Member

Great first PR!

@Gui-FernandesBR Gui-FernandesBR merged commit 6eaf99d into RocketPy-Team:develop Dec 4, 2025
7 checks passed
@erielC erielC deleted the tst/add-fin-flutter-branch-coverage branch December 4, 2025 02:55
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.

2 participants