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: solid_motor.py testing refactors #494

Merged
merged 2 commits into from
Dec 2, 2023
Merged

Conversation

lucasfourier
Copy link
Contributor

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

Current behavior

  1. Currently, test_solidmotor.py creates a motor inside the testing module called 'test_reshape_thrust_curve_asserts_resultant_thrust_curve_correct', which is not adequate. This method also does not contains comments.

  2. Inside the test_function.py testing module the 'test_differentiate' method relies on manually passing the inputs.

New behavior

  1. The solution employed was to create a motor in conftest.py, which was called 'cesaroni_m1670_shifted', and which was used in the test method called 'test_reshape_thrust_curve_asserts_resultant_thrust_curve_correct'. Comments were also made.

  2. It was properly refactored and new comments were made. Also, @pytest.mark.parametrize was introduced to make the test method ready to handle more test cases. This test was introduced in TST: new set of tests  #467.

Breaking change

  • Yes
  • No

Additional information

This PR is part of the effort of reorganizing the test suite. It does so by making sure that the tests which could certainly be classified as unit tests are properly refactored and implemented before the test suite is reorganized. For more info, check #480.

While studying some of the tests that were written, I made some simple changes to conftest.py, test_function.py and test_solidmotor.py.
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (0ef9849) 70.91% compared to head (0180755) 70.91%.

❗ Current head 0180755 differs from pull request most recent head 62c8bb6. Consider uploading reports for the commit 62c8bb6 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #494   +/-   ##
=======================================
  Coverage   70.91%   70.91%           
=======================================
  Files          55       55           
  Lines        9261     9261           
=======================================
  Hits         6567     6567           
  Misses       2694     2694           
Flag Coverage Δ
unittests 70.91% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@phmbressan phmbressan changed the base branch from master to develop November 29, 2023 09:04
@Gui-FernandesBR Gui-FernandesBR changed the title Tst/testing refactor TST: solid_motor.py testing refactors Nov 30, 2023
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.

LGTM.

@MateusStano MateusStano merged commit 7722736 into develop Dec 2, 2023
3 checks passed
@MateusStano MateusStano deleted the tst/testing-refactor branch December 2, 2023 14:54
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.

Sorry for the incovinience but I'd like to say that, actually, I didn't review this one. I was trying to approve other PR but got confused.

@MateusStano I'm trusting your review now.

MateusStano added a commit that referenced this pull request Dec 2, 2023
commit 7722736
Merge: feb3299 62c8bb6
Author: MateusStano <[email protected]>
Date:   Sat Dec 2 11:54:10 2023 -0300

    Merge pull request #494 from RocketPy-Team/tst/testing-refactor

    TST: solid_motor.py testing refactors

commit feb3299
Merge: a9ac31c b27edc3
Author: MateusStano <[email protected]>
Date:   Sat Dec 2 11:46:54 2023 -0300

    Merge pull request #477 from RocketPy-Team/enh/get-attributes

    ENH: Get Instance Attributes

commit b27edc3
Author: MateusStano <[email protected]>
Date:   Wed Nov 29 19:01:32 2023 +0100

    TST: add test

commit dcdd6bd
Merge: 438f4c0 191ab9f
Author: MateusStano <[email protected]>
Date:   Wed Nov 29 18:04:31 2023 +0100

    Merge remote-tracking branch 'origin/develop' into enh/get-attributes

commit 438f4c0
Author: MateusStano <[email protected]>
Date:   Wed Nov 29 17:44:19 2023 +0100

    MNT: improve warnings

commit 62c8bb6
Author: Lint Action <[email protected]>
Date:   Wed Nov 29 03:39:29 2023 +0000

    Fix code style issues with Black

commit 0180755
Author: Lucas <[email protected]>
Date:   Wed Nov 29 00:16:45 2023 -0300

    TST: Simple refactoring

    While studying some of the tests that were written, I made some simple changes to conftest.py, test_function.py and test_solidmotor.py.

commit 04b751d
Author: MateusStano <[email protected]>
Date:   Fri Nov 17 19:08:21 2023 +0100

    BUG: fix time_array assignment in Environment class

commit 1dd5e1e
Author: MateusStano <[email protected]>
Date:   Fri Nov 17 19:08:09 2023 +0100

    ENH refactor get_instance_attributes function to
    filter out methods and protected attributes

commit 931f330
Author: MateusStano <[email protected]>
Date:   Fri Nov 17 16:16:09 2023 +0100

    MNT: add deprecation warnings

commit 2368cab
Author: MateusStano <[email protected]>
Date:   Fri Nov 17 16:15:56 2023 +0100

    ENH: add get attributes function
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.

4 participants