Add unit test suite #4

br3ndonland · 2020-08-31T04:34:41Z

Description

This PR will add a unit test suite to inboard, with a focus on the app modules and start script.

Changes

Add unit tests for inboard app modules

Add utilities module to support basic auth (81d5a90)
Add Starlette Basic Auth middleware to utilities module (a3ffa71, 823408d): This Starlette Basic Auth implementation combines the example from the Starlette authentication docs and starlette/tests/test_authentication.py with the the FastAPI docs on Basic Auth. Starlette will properly verify credentials, and will return a 401 for incorrect credentials.
Add CORS and Basic Auth endpoints to app modules (9e352df)
Add Pytest fixtures to conftest.py (5ba0726, ffbfd5e)
Add unit tests for API endpoints (d6cb27b)

Add unit tests for inboard start script

Upgrade to pytest-mock@^3.3 for type annotations (9470d52): pytest-mock 3.3 is fully type-annotated and provides a MockerFixture for annotating test functions. Thank you, maintainers! 🎉
Add tests for default configuration file paths (aff93b5)
- gunicorn_conf.py
- logging_conf.py
Create temporary configuration files for testing (02a9645)
- tmp_path/gunicorn_conf.py
- tmp_path/logging_conf.py
Add tests for start.py configure_logging() (ce9268d)
Add tests for start.py set_app_module() (d846f54, 5821174)
Add tests for start.py run_pre_start_script() (8c1bc23): This commit will add tests for start.py run_pre_start_script(). Both Python and shell pre-start scripts will be created and tested, along with error verification when attempting to run from an incorrect path.
Add tests for start.py start_server() (92e6b68)
Add docstrings to conftest.py and test_start.py (70b5b91)
Remove reload option when starting Gunicorn server (962176a)
- Commit 92e6b68 updated the behavior of start.start_server() to allow more flexible use of Uvicorn and Gunicorn. One change was the addition of the --reload option for Gunicorn.
- This commit will remove the Gunicorn --reload option.
- Format of the value passed to --reload is unclear. The docs show False, which is formatted as a Python Boolean, but Gunicorn runs from the command line. Passing "False" or "false" (environment variables) both throw gunicorn: error: unrecognized arguments.
- Gunicorn's reloading behavior doesn't seem to work well in general, so if reloading is needed, it's probably best to stick with Uvicorn.
Retain package directory structure in Docker image (3d3333c)
- Prior to this commit, files from the inboard package were broken up and added to the root directory of the Docker image, to allow users to overwrite the /app directory without affecting the running Docker image. This led to some limitations:
  - Complicated COPY commands in the Dockerfile
  - Inability to use package imports in package modules: gunicorn_conf.py had a from start import instead of from inboard.start. This limits the usefulness of inboard as a Python package.
- This commit will restructure the inboard repo to retain the inboard directory in the Docker image, at /app/inboard, simplifying many of the file and module paths as a result.
- This will also simplify operations for downstream applications, because imports stay the same for Python packages with or without Docker.
- There's a risk that users who were overwriting /app when using tiangolo/uvicorn-gunicorn-docker will accidentally overwrite the /app directory here, so the README will be updated to clarify the preferred setup.
- Changes:
  - Update README to encourage users to copy downstream application files into /app/package instead of /app
  - Update set_conf_path() to return str instead of Path, allowing configure_logging() to read the LOGGING_CONF environment variable as a default argument
  - Update file and module paths for new directory structure
  - Add exception handling to inboard/__init__.py version calculation: Although the inboard directory is now copied into the Docker image, the package is still not installed, to allow users to overwrite the pyproject.toml and poetry.lock with their own. The version calculation in inboard/__init__.py depends on __package__, which is not present unless inboard is fully installed as a package.
Test custom app module paths (4a6d6ae)
- This commit will improve the unit tests for start.py set_app_module, and will drop support for redundant environment variables MODULE_NAME and VARIABLE_NAME.
- Changes:
  - Create temporary directory fixture for app modules
    - Create directory with pytest tmp_path_factory
    - Copy inboard.app to temporary directory
    - Scope fixture to entire test session so it is only created once
  - Rewrite test_start.py class TestSetAppModule
  - Drop support for MODULE_NAME and VARIABLE_NAME:
    - This commit began due to some unexpected behavior of start.py set_app_module. The app_module line was only using the MODULE_NAME and VARIABLE_NAME if APP_MODULE was not set. The function could be rewritten with additional conditions, but it's easier and more maintainable to drop the redundant variables.
    - VARIABLE_NAME is vague
    - MODULE_NAME:VARIABLE_NAME is the same as APP_MODULE

Future directions

Commit fe0fe7d will install the Docker Python SDK.
An expanded test suite utilizing the Docker SDK will be added in the future, based on tiangolo/uvicorn-gunicorn-docker.

https://github.com/pytest-dev/pytest-mock https://github.com/pytest-dev/pytest-mock/releases/tag/v3.3.0 > _New in version 3.3.0._ > > pytest-mock is fully type annotated, letting users use static type > checkers to test their code. > > The mocker fixture returns pytest_mock.MockerFixture which can be used > to annotate test functions: > > ```py > from pytest_mock import MockerFixture > > def test_foo(mocker: MockerFixture) -> None: > ... > ``` Other deps updated: - Updating identify (1.4.28 -> 1.4.29) - Updating nodeenv (1.4.0 -> 1.5.0) - Updating typing-extensions (3.7.4.2 -> 3.7.4.3) - Updating pre-commit (2.6.0 -> 2.7.1)

#3

Mypy error: https://mypy.readthedocs.io/en/latest/error_code_list.html ```text Incompatible types in assignment (expression has type "None", variable has type "Dict[str, Any]") [assignment] mypy(error) ``` Source of error: `logconfig_dict` was potentially assigned to `None`. Other changes: - Inboard import will also be corrected to package module path - Remove `# type: ignore` comment on inboard as it is no longer needed

- gunicorn_conf.py - logging_conf.py

- tmp_path/gunicorn_conf.py - tmp_path/logging_conf.py

- Add tests for start.py configure_logging() - Remove unused mocker arguments from conftest.py - Correct error in start.py configure_logging() (getattr() -> hasattr())

This commit will add tests for start.py `run_pre_start_script()`. Both Python and shell pre-start scripts will be created and tested, along with error verification when attempting to run from an incorrect path.

d846f54 Add tests for custom app module variables

- Add `PROCESS_MANAGER` environment variable: previously, the process manager was determined by the environment variable `WITH_RELOAD`. `WITH_RELOAD="true"` would start Uvicorn alone with reloading, or `WITH_RELOAD="false"` would run Uvicorn with Gunicorn as process manager without reloading. - Reconfigure start.py `start_server()` for process managers - Update README for new `PROCESS_MANAGER` environment variable - Update debugger config for new `PROCESS_MANAGER` environment variable - Add tests for start.py `start_server()` - Parametrize tests for all app modules (Base ASGI, FastAPI, Starlette)

Either mocker.patch.dict or copy.deepcopy work for instantiating a copy of the LOGGING_CONFIG dictionary. pytest-mock is already in use, so it's easier and more consistent to stick with pytest-mock.

Other dependencies updated: - more-itertools (8.4.0 -> 8.5.0) - black (19.10b0 -> 20.8b1) - pytest-mock (3.3.0 -> 3.3.1)

92e6b68 This commit will build on 92e6b68, further updating the README with more details on how to use the new `PROCESS_MANAGER` environment variable.

81d5a90 a3ffa71 9e352df ffbfd5e - Add default username and password to make testing easier - Add authentication and error response to Starlette middleware: Starlette will now properly verify credentials, and will return a 401 for incorrect credentials. - Add environment variables to VSCode debugger config - Update conftest.py to match VSCode debugger config - Add instructions to development section of README

92e6b68 Commit 92e6b68 updated the behavior of `start.start_server()` to allow more flexible use of Uvicorn and Gunicorn. One change was the addition of the `--reload` option for Gunicorn. This commit will remove the Gunicorn `--reload` option. Format of the value passed to `--reload` is unclear. The docs show `False`, which is formatted as a Python Boolean, but Gunicorn runs from the command line. Passing `"False"` or `"false"` (environment variables) both throw `gunicorn: error: unrecognized arguments`. Gunicorn's reloading behavior doesn't seem to work well in general, so if reloading is needed, it's probably best to stick with Uvicorn.

Prior to this commit, files from the inboard package were broken up and added to the root directory of the Docker image, to allow users to overwrite the /app directory without affecting the running Docker image. This led to some limitations: - Complicated `COPY` commands in the Dockerfile - Inability to use package imports in package modules: gunicorn_conf.py needed `from start` instead of `from inboard.start`. This limits the usefulness of inboard as a Python package. This commit will restructure the inboard repo to retain the inboard directory in the Docker image, at /app/inboard, simplifying many of the file and module paths as a result. This will also simplify operations for downstream applications, because imports stay the same for Python packages with or without Docker. There's a risk that users who were overwriting /app when using tiangolo/uvicorn-gunicorn will accidentally overwrite the /app directory here, so the README will be updated to clarify the preferred setup. Changes: - Update README to encourage users to copy downstream application files into /app/package instead of /app - Update `set_conf_path()` to return `str` instead of `Path`, allowing `configure_logging()` to read the `LOGGING_CONF` environment variable as a default argument - Update file and module paths for new directory structure - Add exception handling to inboard/`__init__.py` version calculation: Although the inboard directory is now copied into the Docker image, the package is still not installed, to allow users to overwrite the pyproject.toml and poetry.lock with their own. The version calculation in inboard/`__init__.py` depends on `__package__`, which is not present unless inboard is fully installed as a package.

This commit will improve the unit tests for start.py `set_app_module`, and will drop support for redundant environment variables MODULE_NAME and VARIABLE_NAME. This commit began due to some unexpected behavior of start.py `set_app_module`. The `app_module` line was only using the MODULE_NAME and VARIABLE_NAME if APP_MODULE was not set. `app_module = os.getenv("APP_MODULE", f"{module_name}:{variable_name}")` The function could be rewritten with additional conditions, but it's easier and more maintainable to drop the redundant variables. Changes: - Create temporary directory fixture for app modules - Create directory with pytest `tmp_path_factory` - Copy inboard.app to temporary directory - Scope fixture to entire test session so it is only created once - Rewrite test_start.py `class TestSetAppModule` - Drop support for MODULE_NAME and VARIABLE_NAME: - VARIABLE_NAME is vague - MODULE_NAME:VARIABLE_NAME is the same as APP_MODULE

A formatting conflict was preventing the hooks from passing in GitHub Actions CI.

FastAPI is included as one of the Poetry Extras, so it needs to be specified for GitHub Actions, as it is in the Dockerfile.

#4 5ba0726 - Remove the `mocker.patch.object(logger.debug)` lines - Return a `MockerFixture` instead of `logging.Logger` - Remove `# type: ignore[attr-defined] # noqa: E501` mypy and Flake8 comments: now that the logger is a proper `MockerFixture`, pytest and pytest-mock will create the necessary attributes automatically.

#4 5ba0726 - Remove the `mocker.patch.object(logger.debug)` lines - Return a `MockerFixture` instead of `logging.Logger` (TODO: ?) - Remove `# type: ignore[attr-defined] # noqa: E501` mypy and Flake8 comments: now that the logger is a proper `MockerFixture`, pytest and pytest-mock will create the necessary attributes automatically. WIP notes: pytest passing, mypy has errors Having some trouble integrating the mock and the logger. If I make `mock_logger` a `logging.Logger` instance, it doesn't have the mock methods like `assert_has_calls`. However, if I make it `MockerFixture`, it doesn't have access to logger attributes (even after patching). I may need to instantiate a `MagicMock`.

#3 #4 https://docs.python.org/3/library/unittest.mock.html#where-to-patch - Mock the root logger object directly: The project previously had a `mock_logger` pytest fixture in conftest.py that instantiated a root logger with `logging.getLogger()`. The individual log level attributes were then patched with `mocker.patch.object(logger, "debug")`. This was problematic, because Mypy thought that the logger didn't have the attributes, requiring many `# type: ignore[attr-defined]` comments. Instead of `logging.getLogger()`, the root logger object itself will be directly patched whenever it is used for testing. - Remove `# type: ignore[attr-defined]` Mypy comments: now that the `mock_logger` is a proper `MockerFixture`, pytest and pytest-mock will create the necessary attributes automatically. - Improve exception handling when configuring logging: raise `ImportError` if module spec is not created - Simplify `dict` type annotations - Move error messages to error log level instead of debug: makes more sense for errors to be logged as errors - Add `e.__class__.__name__` to log messages when exceptions are raised: helps to have the exception class name - Correct `logging_conf_tmp_path_incorrect_extension` pytest fixture: should actually create a file

#3 #4 https://docs.python.org/3/library/unittest.mock.html#where-to-patch - Mock the root logger object directly: The project previously had a `mock_logger` pytest fixture in conftest.py that instantiated a root logger with `logging.getLogger()`. The individual log level attributes were then patched with `mocker.patch.object(logger, "debug")`. This was problematic, because Mypy thought that the logger didn't have the attributes, requiring many `# type: ignore[attr-defined]` comments. Instead of `logging.getLogger()`, the root logger object itself will be directly patched whenever it is used for testing. - Remove `# type: ignore[attr-defined]` Mypy comments: now that the `mock_logger` is a proper `MockerFixture`, pytest and pytest-mock will create the necessary attributes automatically. - Correct module spec syntax in `set_app_module()`: importlib needs the module path without the app instance name at the end. The way this function was written before, importlib was just returning `None`. inboard should be able to find the app module before continuing, or raise an `ImportError` exception if module spec is not created. - Move error messages to error log level instead of debug: makes more sense for errors to be logged as errors - Add `e.__class__.__name__` to log messages when exceptions are raised: helps to have the exception class name

#3 #4 https://docs.python.org/3/library/unittest.mock.html#where-to-patch - Mock the root logger object directly: The project previously had a `mock_logger` pytest fixture in conftest.py that instantiated a root logger with `logging.getLogger()`. The individual log level attributes were then patched with `mocker.patch.object(logger, "debug")`. This was problematic, because Mypy thought that the logger didn't have the attributes, requiring many `# type: ignore[attr-defined]` comments. Instead of `logging.getLogger()`, the root logger object itself will be directly patched whenever it is used for testing. - Remove `# type: ignore[attr-defined]` Mypy comments: now that the `mock_logger` is a proper `MockerFixture`, pytest and pytest-mock will create the necessary attributes automatically.

#3 #4 3b20619 https://docs.python.org/3/library/unittest.mock.html#where-to-patch - Mock the root logger object directly: The project previously had a `mock_logger` pytest fixture in conftest.py that instantiated a root logger with `logging.getLogger()`. The individual log level attributes were then patched with `mocker.patch.object(logger, "debug")`. This was problematic, because Mypy thought that the logger didn't have the attributes, requiring many `# type: ignore[attr-defined]` comments. Instead of `logging.getLogger()`, the root logger object itself will be directly patched whenever it is used for testing. - Remove `# type: ignore[attr-defined]` Mypy comments: now that the `mock_logger` is a proper `MockerFixture`, pytest and pytest-mock will create the necessary attributes automatically. - Simplify `dict` type annotations - Add `e.__class__.__name__` to log messages when exceptions are raised: helps to have the exception class name

#3 #4 https://docs.python.org/3/library/unittest.mock.html#where-to-patch The project previously had a `mock_logger` pytest fixture in conftest.py that instantiated a root logger with `logging.getLogger()`. Individual log levels were patched with `mocker.patch.object(logger, "debug")`. This was problematic, because Mypy thought that the logger didn't have the attributes, requiring many `# type: ignore[attr-defined]` comments. Instead of `logging.getLogger()`, the root logger object itself will be directly patched whenever it is used for testing. Now that the `mock_logger` is a proper `MockerFixture` within each test, pytest and pytest-mock will create the necessary attributes. This commit will remove the `# type: ignore[attr-defined]` Mypy comments :recycle: :fire: :art: .

br3ndonland added 29 commits August 20, 2020 15:23

Remove example code from template repo

362387f

Install Requests for Starlette test client

8e99b98

Add utilities module to support basic auth

81d5a90

Add Starlette Basic Auth middleware to utilities

a3ffa71

The documentation was not helpful for adding HTTP Basic Auth. I based my middleware on starlette/tests/test_authentication.py instead.

Add CORS and Basic Auth endpoints to app modules

9e352df

- Add CORS allowed origins - Add FastAPI and Starlette app endpoints with HTTP Basic Auth

Add Pytest fixtures to conftest.py

5ba0726

https://docs.pytest.org/en/stable/fixture.html - TestClient instances - Mock logger

Add Pytest fixture for HTTP Basic Auth credentials

ffbfd5e

https://docs.pytest.org/en/stable/fixture.html

Add unit tests for API endpoints

d6cb27b

- CORS - HTTP Basic Auth

Install Docker Python SDK

fe0fe7d

https://docker-py.readthedocs.io/en/stable/

Add README reference to logging PR

4801ae3

#3

Add tests for default configuration file paths

aff93b5

- gunicorn_conf.py - logging_conf.py

Create temporary configuration files for testing

02a9645

- tmp_path/gunicorn_conf.py - tmp_path/logging_conf.py

Add tests for start.py configure_logging()

ce9268d

- Add tests for start.py configure_logging() - Remove unused mocker arguments from conftest.py - Correct error in start.py configure_logging() (getattr() -> hasattr())

Add tests for start.py set_app_module()

d846f54

Add tests for start.py run_pre_start_script()

8c1bc23

This commit will add tests for start.py `run_pre_start_script()`. Both Python and shell pre-start scripts will be created and tested, along with error verification when attempting to run from an incorrect path.

Improve tests for start.py set_app_module()

5821174

d846f54 Add tests for custom app module variables

Use mocker.patch.dict for logging config dict

5607c45

Either mocker.patch.dict or copy.deepcopy work for instantiating a copy of the LOGGING_CONFIG dictionary. pytest-mock is already in use, so it's easier and more consistent to stick with pytest-mock.

Add docstrings to conftest.py and test_start.py

70b5b91

Upgrade FastAPI to 0.61

306d520

Other dependencies updated: - more-itertools (8.4.0 -> 8.5.0) - black (19.10b0 -> 20.8b1) - pytest-mock (3.3.0 -> 3.3.1)

Add info on PROCESS_MANAGER env var to README

d2220ba

92e6b68 This commit will build on 92e6b68, further updating the README with more details on how to use the new `PROCESS_MANAGER` environment variable.

Upgrade Black in pre-commit to 20.8b1

183f8e9

A formatting conflict was preventing the hooks from passing in GitHub Actions CI.

Install FastAPI for GitHub Actions tests workflow

99a01c1

FastAPI is included as one of the Poetry Extras, so it needs to be specified for GitHub Actions, as it is in the Dockerfile.

br3ndonland merged commit 08b7836 into develop Aug 31, 2020

br3ndonland deleted the br3ndonland/tests branch August 31, 2020 04:52

This was referenced Sep 13, 2020

Add test coverage tooling #7

Merged

Increase unit test coverage to 100% #8

Merged

br3ndonland mentioned this pull request Mar 6, 2021

Properly mock logger #28

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit test suite #4

Add unit test suite #4

br3ndonland commented Aug 31, 2020 •

edited

Loading

Add unit test suite #4

Add unit test suite #4

Conversation

br3ndonland commented Aug 31, 2020 • edited Loading

Description

Changes

Add unit tests for inboard app modules

Add unit tests for inboard start script

Future directions

Related

br3ndonland commented Aug 31, 2020 •

edited

Loading