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

Implements environment route tests #37

Merged
merged 10 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
black:
black ./lib
format: flake8 pylint ruff black

lint: flake8 pylint
black:
black ./lib && black ./tests

flake8:
flake8 --ignore E501,E402,F401,W503 ./lib
flake8 --ignore E501,E402,F401,W503,C0414 ./lib && flake8 --ignore E501,E402,F401,W503,C0414 ./tests

GabrielBarberini marked this conversation as resolved.
Show resolved Hide resolved
pylint:
pylint --extension-pkg-whitelist='pydantic' ./lib/*
pylint --extension-pkg-whitelist='pydantic' ./lib && pylint --extension-pkg-whitelist='pydantic' ./tests

ruff:
ruff check --fix

test:
GabrielBarberini marked this conversation as resolved.
Show resolved Hide resolved
python3 -m pytest .

dev:
python3 -m uvicorn lib:app --reload --port 3000
Expand All @@ -19,3 +25,5 @@ clean:

build:
docker build -t infinity-api . --no-cache

@PHONY: black lint flake8 pylint test dev clean build ruff
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
- Install dependencies `python3 -m pip install -r requirements.txt`

## Development
- black ./lib
- pylint --extension-pkg-whitelist='pydantic' ./lib/*
- flake8 --ignore E501,E402,F401,W503 ./lib
- make format
- make test
- make clean
- make build

## Starting the server
- Setup MONGODB_CONNECTION_STRING:
Expand Down
2 changes: 1 addition & 1 deletion lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ def parse_error(error):
return f"{exc_type}: {exc_obj}"


from lib.api import app # pylint: disable=wrong-import-position,cyclic-import
from lib.api import app as app # pylint: disable=wrong-import-position,cyclic-import
21 changes: 6 additions & 15 deletions lib/controllers/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,16 @@ class EnvController:
- CRUD operations over models.Env on the database
"""

def __init__(self, env: Env):
self._env = env

@property
def env(self) -> Env:
return self._env

@env.setter
def env(self, env: Env):
self._env = env

async def create_env(self) -> Union[EnvCreated, HTTPException]:
@staticmethod
async def create_env(env: Env) -> Union[EnvCreated, HTTPException]:
"""
Create a env in the database.

Returns:
views.EnvCreated
"""
try:
async with EnvRepository(self.env) as env_repo:
async with EnvRepository(env) as env_repo:
await env_repo.create_env()
except PyMongoError as e:
logger.error(
Expand Down Expand Up @@ -157,8 +147,9 @@ async def get_rocketpy_env_binary(
f"Call to controllers.environment.get_rocketpy_env_binary completed for Env {env_id}"
)

@staticmethod
async def update_env_by_id(
self, env_id: str
env_id: str, env: Env
) -> Union[EnvUpdated, HTTPException]:
"""
Update a models.Env in the database.
Expand All @@ -173,7 +164,7 @@ async def update_env_by_id(
HTTP 404 Not Found: If the env is not found in the database.
"""
try:
async with EnvRepository(self.env) as env_repo:
async with EnvRepository(env) as env_repo:
await env_repo.update_env_by_id(env_id)
except PyMongoError as e:
logger.error(
Expand Down
8 changes: 4 additions & 4 deletions lib/routes/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async def create_env(env: Env) -> EnvCreated:
``` models.Env JSON ```
"""
with tracer.start_as_current_span("create_env"):
return await EnvController(env).create_env()
return await EnvController.create_env(env)


@router.get("/{env_id}")
Expand All @@ -63,11 +63,11 @@ async def update_env(env_id: str, env: Env) -> EnvUpdated:
```
"""
with tracer.start_as_current_span("update_env"):
return await EnvController(env).update_env_by_id(env_id)
return await EnvController.update_env_by_id(env_id, env)


@router.get(
"/rocketpy/{env_id}",
"/{env_id}/rocketpy",
responses={
203: {
"description": "Binary file download",
Expand Down Expand Up @@ -118,4 +118,4 @@ async def delete_env(env_id: str) -> EnvDeleted:
``` env_id: str ```
"""
with tracer.start_as_current_span("delete_env"):
return await EnvController(env_id).delete_env_by_id(env_id)
return await EnvController.delete_env_by_id(env_id)
23 changes: 13 additions & 10 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,15 +1,7 @@
[build-system]
requires = ["setuptools", "setuptools_scm"]
build-backend = "setuptools.build_meta"

[tool.setuptools.dynamic]
dependencies = {file = ["requirements.txt"]}

[project]
name = "Infinity-API"
version = "2.2.0"
version = "2.3.0"
description = "RESTFULL open API for rocketpy"
dynamic = ["dependencies"]
requires-python = ">=3.12"
authors = [
{name = "Gabriel Barberini", email = "[email protected]"}
Expand All @@ -21,7 +13,7 @@ maintainers = [
readme = "README.md"
keywords = ["rocketpy", "API", "simulation", "rocket", "flight"]
classifiers = [
"Development Status :: Alpha",
"Development Status :: Production",
"Programming Language :: Python"
GabrielBarberini marked this conversation as resolved.
Show resolved Hide resolved
]

Expand Down Expand Up @@ -52,3 +44,14 @@ disable = """
raise-missing-from,
too-many-instance-attributes,
"""

[tool.ruff]
line-length = 79
target-version = "py313"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

79? I usually see 88 instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the API development we set PEP-8 as a north


GabrielBarberini marked this conversation as resolved.
Show resolved Hide resolved
Gui-FernandesBR marked this conversation as resolved.
Show resolved Hide resolved
[tool.ruff.lint]
select = ["E", "F", "N", "Q"]
ignore = ["N815", "E501", "Q000", "E402"]
fixable = [
"F401",
]
3 changes: 3 additions & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
flake8
pylint
ruff
114 changes: 114 additions & 0 deletions tests/test_routes/test_environment_route.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
import pytest
from fastapi.testclient import TestClient
from unittest.mock import patch
from lib.models.environment import Env
from lib.controllers.environment import EnvController
from lib.views.environment import (
EnvCreated,
EnvUpdated,
EnvDeleted,
EnvSummary,
)
from lib import app

client = TestClient(app)


@pytest.fixture
def mock_env():
return Env(latitude=0, longitude=0)


@pytest.fixture
def mock_env_summary():
return EnvSummary()


def test_create_env(mock_env):
with patch.object(
EnvController, "create_env", return_value=EnvCreated(env_id="123")
) as mock_create_env:
response = client.post(
"/environments/", json={"latitude": 0, "longitude": 0}
)
assert response.status_code == 200
assert response.json() == {
"env_id": "123",
"message": "Environment successfully created",
}
mock_create_env.assert_called_once_with(Env(latitude=0, longitude=0))

Copy link
Contributor

@coderabbitai coderabbitai bot Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error case testing for CRUD operations.

The current tests only cover successful scenarios. Consider adding tests for:

  • Invalid input data
  • Non-existent resources
  • Server errors
  • Authorization failures

Example implementation for error cases:

def test_create_env_invalid_data():
    response = client.post(
        "/environments/",
        json={"latitude": "invalid", "longitude": 0}
    )
    assert response.status_code == 422

def test_read_env_not_found():
    with patch.object(
        EnvController,
        "get_env_by_id",
        side_effect=KeyError("Not found")
    ):
        response = client.get("/environments/nonexistent")
        assert response.status_code == 404

Also applies to: 42-52, 54-71, 73-86

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pertinent, will comply.

cc @Gui-FernandesBR, @aasitvora99 , @luimot any other test category you think worth adding to the routes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


def test_read_env(mock_env):
with patch.object(
EnvController, "get_env_by_id", return_value=mock_env
) as mock_read_env:
response = client.get("/environments/123")
assert response.status_code == 200
expected_content = mock_env.model_dump()
expected_content["date"] = expected_content["date"].isoformat()
assert response.json() == expected_content
mock_read_env.assert_called_once_with("123")


def test_update_env():
with patch.object(
EnvController,
"update_env_by_id",
return_value=EnvUpdated(env_id="123"),
) as mock_update_env:
response = client.put(
"/environments/123", json={"longitude": 1, "latitude": 1}
)
assert response.status_code == 200
assert response.json() == {
"env_id": "123",
"message": "Environment successfully updated",
}
mock_update_env.assert_called_once_with(
"123", Env(latitude=1, longitude=1)
)


def test_delete_env():
with patch.object(
EnvController,
"delete_env_by_id",
return_value=EnvDeleted(env_id="123"),
) as mock_delete_env:
response = client.delete("/environments/123")
assert response.status_code == 200
assert response.json() == {
"env_id": "123",
"message": "Environment successfully deleted",
}
mock_delete_env.assert_called_once_with("123")


def test_simulate_env(mock_env_summary):
with patch.object(
EnvController, "simulate_env", return_value=mock_env_summary
) as mock_simulate_env:
response = client.get("/environments/123/summary")
assert response.status_code == 200
expected_content = mock_env_summary.model_dump()
expected_content["date"] = expected_content["date"].isoformat()
expected_content["local_date"] = expected_content[
"local_date"
].isoformat()
expected_content["datetime_date"] = expected_content[
"datetime_date"
].isoformat()
assert response.json() == expected_content
mock_simulate_env.assert_called_once_with("123")


def test_read_rocketpy_env(mock_env):
with patch.object(
EnvController, "get_rocketpy_env_binary", return_value=b'rocketpy'
) as mock_read_rocketpy_env:
response = client.get("/environments/123/rocketpy")
assert response.status_code == 203
assert response.content == b'rocketpy'
assert response.headers["content-type"] == "application/octet-stream"
mock_read_rocketpy_env.assert_called_once_with("123")
Loading