Skip to content

Create Unit Tests for analyze_unsteady_trim in trim.py#183

Open
20086080 wants to merge 3 commits into
camUrban:mainfrom
20086080:Issue-137
Open

Create Unit Tests for analyze_unsteady_trim in trim.py#183
20086080 wants to merge 3 commits into
camUrban:mainfrom
20086080:Issue-137

Conversation

@20086080
Copy link
Copy Markdown
Contributor

Description

Add unit tests for analyze_unsteady_trim in trim.py covering parameter validation and operating point bounds validation.

These tests improve confidence in the trim analysis interface by verifying that invalid inputs are correctly rejected before optimization or solver execution begins.

Motivation

analyze_unsteady_trim contains extensive validation logic for user-supplied parameters and operating point constraints. These checks are critical because they prevent invalid optimization searches and ensure that trim analysis is only performed on well-defined UnsteadyProblem configurations.

The function was previously flagged with # TEST comments but did not have dedicated unit test coverage. Adding focused validation tests makes future refactoring safer and helps ensure that documented parameter requirements remain enforced.

Relevant Issues

Resolves unit testing component of #137

Changes

Added dedicated unit tests for analyze_unsteady_trim in test_unsteady_trim.py.

The following validation behavior is now tested:

  1. test_problem_validation - Invalid problem parameter type raises TypeError.

  2. test_multiple_airplane_movements_validation - Multiple AirplaneMovement objects raise ValueError.

  3. test_boundsVCg__E_validation - Test boundsVCg__E parameter validation:

  • Invalid type
  • Incorrect tuple length
  • Non-numeric values
  • Descending bounds
  • Non-positive values
  1. test_alpha_bounds_validation - Test alpha_bounds parameter validation:
  • Invalid type
  • Incorrect tuple length
  • Non-numeric values
  • Descending bounds
  1. test_beta_bounds_validation - Test beta_bounds parameter validation:
  • Invalid type
  • Incorrect tuple length
  • Non-numeric values
  • Descending bounds
  1. test_boundsExternalFX_W_validation - Test boundsExternalFX_W parameter validation:
  • Invalid type
  • Incorrect tuple length
  • Non-numeric values
  • Descending bounds
  1. test_objective_cut_off_validation - Test objective_cut_off parameter validation.

  2. test_num_calls_validation - Test num_calls parameter validation.

  3. test_base_operating_point_parameter_bounds_validation - Test that the base operating point values must lie within the supplied boundss:

  • vCg__E

  • alpha

  • beta

  • externalFX_W

    must lie within the user-supplied search bounds.

Testing Methodology

Tests use existing fixtures from tests.unit.fixtures.problem_fixtures to construct valid UnsteadyProblem instances.

The testing approach focuses on:

  • Parameter validation
  • Boundary condition validation
  • Verification of documented input requirements
  • Error handling through assertRaises

The tests intentionally avoid executing the optimization routines or aerodynamic solver, allowing validation logic to be tested in isolation as true unit tests.

Dependency Updates

None

Change Magnitude

Minor: Small change such as a bug fix, small enhancement, or documentation update.

Checklist (check each item when completed or not applicable)

  • I am familiar with the current contribution guidelines.
  • PR description links all relevant issues and follows this template.
  • My branch is based on main and is up to date with the upstream main branch.
  • All calculations use S.I. units.
  • Code is formatted with black (line length = 88).
  • Code is well documented with block comments where appropriate.
  • Any external code, algorithms, or equations used have been cited in comments or docstrings.
  • All new modules, classes, functions, and methods have docstrings in reStructuredText format, and are formatted using docformatter (--in-place --black). See the style guide for type hints and docstrings for more details.
  • All new classes, functions, and methods in the pterasoftware package use type hints. See the style guide for type hints and docstrings for more details.
  • If any major functionality was added or significantly changed, I have added or updated tests in the tests package.
  • Code locally passes all tests in the tests package.
  • This PR passes the ReadTheDocs build check (this runs automatically with the other workflows).
  • This PR passes the ascii-only, black, codespell, docformatter, isort, and pre-commit-hooks GitHub actions.
  • This PR passes the mypy GitHub action.
  • This PR passes all the tests GitHub actions.

@20086080 20086080 requested a review from camUrban as a code owner May 31, 2026 00:59
@codecov
Copy link
Copy Markdown

codecov Bot commented May 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.42%. Comparing base (fb08e9f) to head (1fd6657).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
+ Coverage   91.77%   92.42%   +0.64%     
==========================================
  Files          37       37              
  Lines        7234     7234              
==========================================
+ Hits         6639     6686      +47     
+ Misses        595      548      -47     

☔ 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.

@camUrban camUrban added the maintenance Improvements or additions to documentation, testing, or robustness label May 31, 2026
@camUrban camUrban added this to the v5.1.0 milestone May 31, 2026
Copy link
Copy Markdown
Owner

@camUrban camUrban left a comment

Choose a reason for hiding this comment

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

Hi @20086080, thanks for the work here. Your tests look great! I left a few suggestions for those, but the bigger thing is all the .idea file changes. We still don't want those modified or deleted or added to .gitignore. I think the best way to fix it is to follow the steps 1-6 from my comment here. The only thing to do different this time is to include the file names of all the .idea files and also .gitignore for step 2.

It seems like the steps I provided last time didn't quite work in getting your git install to ignore these. Therefore, in the future, I'd recommend not using git add . before committing. Instead, you can git add <file_1> <file_2> ... with the files you actually want to change, which should prevent your next git commit -m "<some message>" from picking up any extraneous changes. 😊

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't delete.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't delete.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't delete.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't delete.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't delete.

Comment thread .idea/watcherTasks.xml
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Don't delete.

@@ -0,0 +1,294 @@
import unittest
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please add a module docstring (see other unit test files for the proper format). Also, you'll want to add this file's docstring to the list of docstrings in tests/unit/init.py and also import it there (in alphabetical order).

boundsExternalFX_W=(10.0, -10.0),
)

def test_objective_cut_off_validation(self):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It might be good to also add an assertRaises test on negative objective_cut_off values as well.

objective_cut_off=0.0,
)

def test_num_calls_validation(self):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

For num_calls, we also want to throw out non-integer values and negative values.

Comment thread .gitignore
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

No need to add anything to this file.

@20086080
Copy link
Copy Markdown
Contributor Author

20086080 commented Jun 1, 2026

Hi @camUrban

I have added in the 2 test changes and doc string.
Unfortunately I am still a bit stuck with the .idea/ files. Am on Step 4 of #169 (comment). Step 1 - 3 were fine (for Step 2 I individually added in the files in the .git/info/exclude).
For step 4 - This is what my git status is showing me. Please let me know what best to do. The changes in the test file were committed before I started trying to fix the .idea/ files.

Changes to be committed:
(use "git restore --staged ..." to unstage)
new file: .idea/.gitignore
new file: .idea/.name
new file: .idea/PteraSoftware.iml
new file: .idea/codeStyles/Project.xml
new file: .idea/codeStyles/codeStyleConfig.xml
new file: .idea/copilot.data.migration.agent.xml
new file: .idea/copilot.data.migration.ask.xml
new file: .idea/copilot.data.migration.ask2agent.xml
new file: .idea/copilot.data.migration.edit.xml
new file: .idea/dictionaries/project.xml
new file: .idea/icon.svg
new file: .idea/inspectionProfiles/Ptera_Software_Default.xml
new file: .idea/inspectionProfiles/profiles_settings.xml
new file: .idea/jsonCatalog.xml
new file: .idea/markdown.xml
new file: .idea/misc.xml
new file: .idea/modules.xml
new file: .idea/other.xml
new file: .idea/scopes/to_inspect.xml
new file: .idea/statistic.xml
new file: .idea/vcs.xml
new file: .idea/watcherTasks.xml

Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
modified: .idea/dictionaries/project.xml

@camUrban
Copy link
Copy Markdown
Owner

camUrban commented Jun 1, 2026

Hi @20086080, thanks for reaching back out. Don't worry, you are mostly there! One quick question to be sure we're on the same page:

When you ran step 1 from the comment, did you run it verbatim, or did you substitute the old branch name with the current branch? You can check by looking at the top few lines in your git status output. They should say what branch you are on. We want to be on Issue-137.

Once you're sure you are on the right branch, the next step is to also add the .gitignore file to the changes you'll revert. To do that, run git checkout HEAD~1 -- .gitignore. Now, the "Changes to be committed list: in git status should look something like:

modified file: .gitignore
new file: .idea/.gitignore
new file: .idea/.name
new file: .idea/PteraSoftware.iml
new file: .idea/codeStyles/Project.xml
new file: .idea/codeStyles/codeStyleConfig.xml
new file: .idea/copilot.data.migration.agent.xml
new file: .idea/copilot.data.migration.ask.xml
new file: .idea/copilot.data.migration.ask2agent.xml
new file: .idea/copilot.data.migration.edit.xml
new file: .idea/dictionaries/project.xml
new file: .idea/icon.svg
new file: .idea/inspectionProfiles/Ptera_Software_Default.xml
new file: .idea/inspectionProfiles/profiles_settings.xml
new file: .idea/jsonCatalog.xml
new file: .idea/markdown.xml
new file: .idea/misc.xml
new file: .idea/modules.xml
new file: .idea/other.xml
new file: .idea/scopes/to_inspect.xml
new file: .idea/statistic.xml
new file: .idea/vcs.xml
new file: .idea/watcherTasks.xml

Once you've done that, you can just run a normal commit and push:

git commit -m "Restore .gitignore and .idea"
git push

After that, let me know if you have any other issues walking through step 8!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Improvements or additions to documentation, testing, or robustness

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants