Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: Reorganize only the test_environment.py tests into Integration/Unit Tests #577

Merged
merged 11 commits into from
Jun 29, 2024

Conversation

lucasfourier
Copy link
Collaborator

Pull request type

  • Code changes (bugfix, features)
  • Code maintenance (refactoring, formatting, tests)
  • ReadMe, Docs and GitHub updates
  • Other (please describe):

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
  • CHANGELOG.md has been updated (if relevant)

Current behavior

Currently, the folder tests/integration is empty.

New behavior

Integration tests folder is no longer empty and contains integration tests. They are not new, they were simply moved to the right place.

Breaking change

  • Yes
  • No

Additional information

image

@lucasfourier lucasfourier added the Tests Regarding Tests label Mar 16, 2024
@lucasfourier lucasfourier requested a review from a team as a code owner March 16, 2024 22:07
Copy link

codecov bot commented Mar 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.92%. Comparing base (5f57e5f) to head (622ae81).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #577      +/-   ##
===========================================
- Coverage    73.73%   71.92%   -1.81%     
===========================================
  Files           70       70              
  Lines        10334    10334              
===========================================
- Hits          7620     7433     -187     
- Misses        2714     2901     +187     

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

@Gui-FernandesBR
Copy link
Member

@lucasfourier , why moving just some of the tests of the tests from tests/test_environment.py instead of all of them?
What does set these tests apart?

@lucasfourier
Copy link
Collaborator Author

Because the tests that have been moved can be defined as integration tests as opposed to the remaining ones. The fundamental trait is the communication with external dependencies.

@Gui-FernandesBR Gui-FernandesBR changed the title Moving tests which are integration tests to the integration folder TST: Reorganize Integration Tests into Dedicated Folder Mar 22, 2024
@Gui-FernandesBR Gui-FernandesBR added this to the Release v1.X.0 milestone Mar 22, 2024
@Gui-FernandesBR
Copy link
Member

@lucasfourier Can you please re-work on this branch to move all the integration tests at once instead of just some of the tests?

@lucasfourier lucasfourier changed the title TST: Reorganize Integration Tests into Dedicated Folder TST: Reorganize only the test_environment.py tests into Integration/Unit Tests May 21, 2024
@Gui-FernandesBR
Copy link
Member

@lucasfourier what is the reason for these methods be considered integration and not unit tests? They were moved from unit tests to the integration tests.

  1. test_environment_export_environment_exports_valid_environment_json
  2. test_date_aware_set_date_saves_custom_timezone
  3. test_date_naive_set_date_saves_utc_timezone_by_default

The definition of unit and integration are not clear enough. I think it is important that you write down these definition before proceeding with any changes in this PR. This should take a few minutes, but save you from questions like the 3 above.

@lucasfourier
Copy link
Collaborator Author

@Gui-FernandesBR You are 100% right. For some reason i thought pytz made some http request at some point, but it does not. Every test there is an unit test, thank you.

@lucasfourier
Copy link
Collaborator Author

Tests with connection to API'S/interfaces were moved into the integration folder and therefore are only run with --runslow flag, just for the record.

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

Good work! @lucasfourier

@Gui-FernandesBR Gui-FernandesBR merged commit 0e9cdac into develop Jun 29, 2024
10 of 11 checks passed
@Gui-FernandesBR Gui-FernandesBR deleted the tst/unit-integration-tests branch June 29, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tests Regarding Tests
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants