feat: align test suits #39

AlexTheWizardL · 2024-07-05T06:10:57Z

This PR covers changes that align test suits to a common standard.

Removed docstrings from test suits and renamed them instead. Example:

from:

def test_keep_extra_columns() -> None:
    """Test dropping columns not in the required or optional lists."""

to:

def test_drop_extra_columns_with_keep_extra_columns_false():

Deleted useless branches of code that are not possible by the system, specifically we can not have entries with the same ID.
Example (this code is removed):

elif len(result) > 1:
            raise ValueError(f"Multiple paths found for account category id: {id}")

Removed useless type hints for the fixtures and tests suits. For further details, see the discussions on type annotations for fixtures, the pytest documentation and the Django project.

codecov · 2024-07-05T06:17:43Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Flag	Coverage Δ
unittests	`93.86% <100.00%> (+0.72%)`	⬆️

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

Files with missing lines	Coverage Δ
cashctrl_api/cashed_client.py	`94.02% <ø> (+1.83%)`	⬆️
cashctrl_api/client.py	`92.95% <100.00%> (ø)`

lasuk

Thanks for streamlining unit tests. I left a few minor comments. Back to you @AlexTheWizardL .

lasuk · 2024-07-09T05:30:08Z

tests/test_file_up_and_download.py

@@ -6,13 +6,11 @@


 def random_word(length: int) -> str:
-    """Generate a random word using lowercase letters."""


I found this comment helpful and would leave it.

lasuk · 2024-07-09T05:45:50Z

tests/test_cached_accounts.py

@@ -7,101 +7,83 @@


 @pytest.fixture(scope="module")


Any benefit of using a fixture?

The only lines of code are reduced since no need to define a class instance in every test suit

Decided on the meeting to remove fixture for this case

lasuk · 2024-07-09T05:45:52Z

tests/test_cached_accounts.py

@@ -7,101 +7,83 @@


 @pytest.fixture(scope="module")
-def cc_client() -> CachedCashCtrlClient:
+def cc_client():
    return CachedCashCtrlClient()


 @pytest.fixture(scope="module")


Any benefit of using a fixture?

lasuk · 2024-07-09T05:45:54Z

tests/test_cached_accounts.py

    return CachedCashCtrlClient()


 @pytest.fixture(scope="module")
-def accounts() -> pd.DataFrame:
+def accounts():
    """Explicitly call the base class method to circumvent the cache."""


This should be a comment, not a docstring.
Applies to all test_cached_....

lasuk · 2024-07-09T05:45:56Z

tests/test_cached_accounts.py

    return CachedCashCtrlClient()


 @pytest.fixture(scope="module")
-def accounts() -> pd.DataFrame:
+def accounts():
    """Explicitly call the base class method to circumvent the cache."""
    cc_client = CachedCashCtrlClient()


Is this in line with the comment 'Explicitly call the base class method to circumvent the cache.'?

Also applies to test_cached_accounts.py, test_cached_tax_rates.py, test_cached_tax_rates.py

lasuk · 2024-07-09T06:18:00Z

tests/test_list_directory.py

-def mock_directory(tmp_path: Path) -> Path:
+def mock_directory(tmp_path: Path):


I found a return type hint for fixtures and other local helper functions helpful.
Let's discuss which is better

…tead, remove useless code snippets

lasuk · 2024-09-03T15:45:06Z

Merging. Thanks for the changes!

AlexTheWizardL requested a review from lasuk July 5, 2024 06:11

lasuk force-pushed the feat/align-test-suits branch from 8503893 to ccf9f17 Compare July 9, 2024 05:47

lasuk reviewed Jul 9, 2024

View reviewed changes

AlexTheWizardL and others added 5 commits August 5, 2024 11:14

feat: remove useless type hints, remove comments and rename tests ins…

c66febd

…tead, remove useless code snippets

fix: align test suits missed type hints

cc49ce7

refactor: Align mock directories in tests

d312393

feat: use comments instead of docstring, move nack valuable docstrings

ee37248

fix: call base class method to circumvent the cache

05262a7

AlexTheWizardL force-pushed the feat/align-test-suits branch from 6a393c9 to 05262a7 Compare August 5, 2024 08:14

lasuk merged commit 2a9a8bd into main Sep 3, 2024
6 checks passed

lasuk deleted the feat/align-test-suits branch September 3, 2024 15:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: align test suits #39

feat: align test suits #39

AlexTheWizardL commented Jul 5, 2024

codecov bot commented Jul 5, 2024 •

edited

Loading

lasuk left a comment

lasuk Jul 9, 2024

lasuk Jul 9, 2024

AlexTheWizardL Jul 9, 2024

AlexTheWizardL Jul 10, 2024

lasuk Jul 9, 2024

lasuk Jul 9, 2024

lasuk Jul 9, 2024

lasuk Jul 9, 2024

lasuk commented Sep 3, 2024

		@@ -6,13 +6,11 @@


		def random_word(length: int) -> str:
		"""Generate a random word using lowercase letters."""

		def mock_directory(tmp_path: Path) -> Path:
		def mock_directory(tmp_path: Path):

feat: align test suits #39

feat: align test suits #39

Conversation

AlexTheWizardL commented Jul 5, 2024

This PR covers changes that align test suits to a common standard.

codecov bot commented Jul 5, 2024 • edited Loading

Codecov Report

lasuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasuk commented Sep 3, 2024

codecov bot commented Jul 5, 2024 •

edited

Loading