From cf391cf10a76654f8655f42dcf271bb4bcd9336c Mon Sep 17 00:00:00 2001 From: zhavir Date: Sat, 27 Aug 2022 13:45:26 +0200 Subject: [PATCH] test(type-checking): enforce type checking on tests, user setup.cfg (#12) --- .coveragerc | 2 -- .github/workflows/run-tests.yml | 2 +- .pre-commit-config.yaml | 19 +++++++++++++++++ README.md | 12 +++++++++++ requirements-dev.txt | 4 +++- setup.cfg | 21 +++++++++++++++++++ src/app/main.py | 2 +- src/app/routers/models/requests.py | 2 +- src/app/settings.py | 2 +- src/app/utils/get_string_chars.py | 2 +- .../integration/test_e2e_password_generate.py | 21 +++++++++++-------- .../test_randomizer_provider.py | 2 +- .../test_password_generate_router.py | 4 ++-- 13 files changed, 75 insertions(+), 20 deletions(-) delete mode 100644 .coveragerc create mode 100644 .pre-commit-config.yaml create mode 100644 setup.cfg diff --git a/.coveragerc b/.coveragerc deleted file mode 100644 index 18b8a4b..0000000 --- a/.coveragerc +++ /dev/null @@ -1,2 +0,0 @@ -[run] -omit = */main.py \ No newline at end of file diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 9c610d2..be431fd 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -25,7 +25,7 @@ jobs: echo "PYTHONPATH=src/" >> $GITHUB_ENV - name: Test with pytest run: | - pytest --cov=src/app --cov=src/tests --cov-report xml:coverage.xml src + pytest - name: Upload coverage if: ${{ github.ref == 'refs/heads/main' }} diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..9d4933b --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,19 @@ +# NOTE: The actions below will be executed in the order of definition +repos: + # Remove unused imports + - repo: https://github.com/PyCQA/autoflake + rev: v1.4 + hooks: + - id: autoflake + args: [ "--in-place", "--remove-all-unused-imports" ] + # Format imports + - repo: https://github.com/pycqa/isort + rev: 5.10.1 + hooks: + - id: isort + # Format code + - repo: https://github.com/google/yapf + rev: v0.32.0 + hooks: + - id: yapf + args: ["-pri", "src"] diff --git a/README.md b/README.md index ec96c67..a4e2b7a 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,16 @@ Openapi specifications available at: http://localhost:9001/docs/ #### Registry Docker registry available at: https://hub.docker.com/r/zhavir/python-assessment +#### 4. Pre-commit hooks + +I used pre-commit hooks to format the code with `yapf` and optimize the imports using `isort`. + +To activate the hooks you have to run the following command: + +```bash +pre-commit install +``` + ## Best practices followed In the first step, I used a multi-layer pattern approach just because the domain was very tiny and does not justify a domain driven approach ( for example by following the hexagonal architecture ). @@ -99,6 +109,8 @@ I followed those principles while modelling the classes: * Open–closed principle * Single-responsibility principle +Due to the fact python is not a typed language, I've adopted MyPy in order to enforce the type checking on tests run + For the testing, I used to cover everything with the unit tests. Meanwhile, I've tested only some happy path with the integration tests. This because I've decided to follow the testing pyramid principle. Moreover, I tried to follow the 100% test coverage objective diff --git a/requirements-dev.txt b/requirements-dev.txt index 487c502..d6be21c 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -3,4 +3,6 @@ httpx pytest pytest-asyncio pytest-cov -pytest-mock \ No newline at end of file +pytest-mock +pytest-mypy +pre-commit \ No newline at end of file diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 0000000..5bbe140 --- /dev/null +++ b/setup.cfg @@ -0,0 +1,21 @@ +[tool:pytest] +addopts = + --mypy + --cov=src/app + --cov=src/tests + --cov-report xml:coverage.xml + src + +[isort] +include_trailing_comma=true +use_parentheses=true +line_length=120 +multi_line_output=3 + +[mypy] +python_version = 3.10 +ignore_missing_imports = True +plugins = pydantic.mypy + +[coverage:run] +omit = */main.py \ No newline at end of file diff --git a/src/app/main.py b/src/app/main.py index 2cca797..8ac3bd5 100644 --- a/src/app/main.py +++ b/src/app/main.py @@ -38,7 +38,7 @@ def custom_openapi(): return app.openapi_schema -app.openapi = custom_openapi +app.openapi = custom_openapi # type: ignore if __name__ == "__main__": parser = argparse.ArgumentParser() diff --git a/src/app/routers/models/requests.py b/src/app/routers/models/requests.py index 0a431df..3e120e5 100644 --- a/src/app/routers/models/requests.py +++ b/src/app/routers/models/requests.py @@ -4,7 +4,7 @@ class GeneratePasswordRequest(BaseModel): - password_length: conint(gt=0, le=200) = settings.default_password_length + password_length: conint(gt=0, le=200) = settings.default_password_length # type: ignore has_numbers: bool = settings.default_has_numbers has_lowercase_chars: bool = settings.default_has_lowercase_chars has_uppercase_chars: bool = settings.default_has_uppercase_chars diff --git a/src/app/settings.py b/src/app/settings.py index cddb562..becd8ca 100644 --- a/src/app/settings.py +++ b/src/app/settings.py @@ -4,7 +4,7 @@ class Settings(BaseSettings): - default_password_length: conint(gt=0, le=200) = 10 + default_password_length: conint(gt=0, le=200) = 10 # type: ignore default_has_numbers: bool = True default_has_lowercase_chars: bool = True default_has_uppercase_chars: bool = True diff --git a/src/app/utils/get_string_chars.py b/src/app/utils/get_string_chars.py index 89fbb3e..1deb22d 100644 --- a/src/app/utils/get_string_chars.py +++ b/src/app/utils/get_string_chars.py @@ -9,7 +9,7 @@ async def get_string_chars( uppercase_letters: bool, special_chars: bool, ) -> List[str]: - allowed_chars = [] + allowed_chars: List[str] = [] if numbers: allowed_chars.extend(string.digits) diff --git a/src/tests/integration/test_e2e_password_generate.py b/src/tests/integration/test_e2e_password_generate.py index 3ea28f2..5487bd9 100644 --- a/src/tests/integration/test_e2e_password_generate.py +++ b/src/tests/integration/test_e2e_password_generate.py @@ -11,7 +11,7 @@ 'has_lowercase_chars', 'has_uppercase_chars', 'has_special_chars', - 'expected_match' + 'expected_match', ], [ (20, True, False, False, False, r'^\d+$'), @@ -29,17 +29,20 @@ async def test_e2e_generate_password( has_lowercase_chars: bool, has_uppercase_chars: bool, has_special_chars: bool, - expected_match: str + expected_match: str, ): async with mocked_client as client: - response = await client.post("/api/v1/passwords/generate/", json={ - "password_length": password_length, - "has_numbers": has_numbers, - "has_lowercase_chars": has_lowercase_chars, - "has_uppercase_chars": has_uppercase_chars, - "has_special_chars": has_special_chars, - }) + response = await client.post( + "/api/v1/passwords/generate/", + json={ + "password_length": password_length, + "has_numbers": has_numbers, + "has_lowercase_chars": has_lowercase_chars, + "has_uppercase_chars": has_uppercase_chars, + "has_special_chars": has_special_chars, + } + ) assert response.status_code == 200 body = response.json() diff --git a/src/tests/unit/test_providers/test_randomizer_provider.py b/src/tests/unit/test_providers/test_randomizer_provider.py index 31f432c..37c5fbc 100644 --- a/src/tests/unit/test_providers/test_randomizer_provider.py +++ b/src/tests/unit/test_providers/test_randomizer_provider.py @@ -14,7 +14,7 @@ (["test", "test2"], 1), (list(string.digits), 5), (list(string.digits), 15), - ] + ], ) @pytest.mark.asyncio async def test_generate_random_sample(values: List[str], length: int): diff --git a/src/tests/unit/test_routers/test_password_generate_router.py b/src/tests/unit/test_routers/test_password_generate_router.py index 1058a3b..9248f1b 100644 --- a/src/tests/unit/test_routers/test_password_generate_router.py +++ b/src/tests/unit/test_routers/test_password_generate_router.py @@ -8,7 +8,7 @@ async def test_generate_password( mocked_client: AsyncClient, mocked_password_generator_service: AsyncMock, - mocked_randomizer_provider, + mocked_randomizer_provider, ): mocked_password_generator_service.return_value.generate_password = AsyncMock(return_value="something") @@ -41,7 +41,7 @@ async def test_generate_password( async def test_generate_password_but_input_is_not_valid( mocked_client: AsyncClient, mocked_password_generator_service: AsyncMock, - mocked_randomizer_provider, + mocked_randomizer_provider, body: dict, ): async with mocked_client as client: