Skip to content

Commit

Permalink
chore: Enable pylint in tests folder
Browse files Browse the repository at this point in the history
Enable pylint in the tests folder. The pylint-pytest plugin is used to prevent false-positives for
fixtures. There are still some false-positives for unused arguments in methods and classes that we
mock, so corresponding comments to ignore these errors are added. This was also used to fix some
valid issues that pylint found (like duplicate and unnecessary imports).

Closes #544
  • Loading branch information
zusorio committed Oct 29, 2024
1 parent fc151f9 commit 254d430
Show file tree
Hide file tree
Showing 36 changed files with 92 additions and 57 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ jobs:
- name: Install pre-commit
run: |-
python -m pip install pre-commit
- name: Install pylint
- name: Install dependencies
run: |-
python -m pip install pylint
cd backend/ && python -m pip install '.[dev]' && cd ..
- name: Run pre-commit
run: |-
pre-commit run --all-files
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ repos:
args: [--rcfile=./backend/pyproject.toml]
language: system
types: [python]
files: '^backend/capellacollab'
files: '^backend'
exclude: '^backend/capellacollab/alembic/'
- repo: local
hooks:
Expand Down
12 changes: 3 additions & 9 deletions backend/capellacollab/core/database/decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,19 @@ def __init__(self, pydantic_model: t.Type[pydantic.BaseModel]):
super().__init__()
self.pydantic_model = pydantic_model

def process_bind_param(
self, value, dialect # pylint: disable=unused-argument
):
def process_bind_param(self, value, dialect):
"""Convert a pydantic object to JSONB."""
if value is None:
return None
return value.model_dump()

def process_literal_param(
self, value, dialect # pylint: disable=unused-argument
):
def process_literal_param(self, value, dialect):
"""Convert a literal pydantic object to JSONB."""
if value is None:
return None
return value.model_dump()

def process_result_value(
self, value, dialect # pylint: disable=unused-argument
):
def process_result_value(self, value, dialect):
"""Convert JSONB to a pydantic object."""
if value is None:
return None
Expand Down
4 changes: 3 additions & 1 deletion backend/capellacollab/core/database/migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def get_eclipse_session_configuration() -> (


def create_capella_tool(db: orm.Session) -> tools_models.DatabaseTool:
registry = config.docker.sessions_registry
registry: str = config.docker.sessions_registry

capella = tools_models.CreateTool(
name="Capella",
Expand All @@ -194,6 +194,7 @@ def create_capella_tool(db: orm.Session) -> tools_models.DatabaseTool:
capella_database = tools_crud.create_tool(db, capella)

for capella_version_name in ("5.0.0", "5.2.0", "6.0.0", "6.1.0", "7.0.0"):
# pylint: disable=unsupported-membership-test
if "localhost" in registry:
docker_tag = f"{capella_version_name}-latest"
else:
Expand Down Expand Up @@ -344,6 +345,7 @@ def create_jupyter_tool(db: orm.Session) -> tools_models.DatabaseTool:
def create_tools(db: orm.Session):
create_capella_tool(db)

# pylint: disable=unsupported-membership-test
if "localhost" in config.docker.sessions_registry:
create_papyrus_tool(db)
create_jupyter_tool(db)
Expand Down
1 change: 1 addition & 0 deletions backend/capellacollab/sessions/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def get_session_by_id(

def count_sessions(db: orm.Session) -> int:
count = db.scalar(
# pylint: disable=not-callable
sa.select(sa.func.count()).select_from(models.DatabaseSession)
)
return count if count else 0
Expand Down
1 change: 0 additions & 1 deletion backend/generate_git_archival.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import pathlib
import subprocess
import typing as t


def run_git_command(cmd: list[str]):
Expand Down
3 changes: 2 additions & 1 deletion backend/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ dev = [
"isort",
"mypy",
"pylint",
"pylint-pytest",
"pytest",
"testcontainers",
"httpx",
Expand Down Expand Up @@ -222,7 +223,7 @@ extension-pkg-whitelist = "pydantic" # https://github.com/pydantic/pydantic/issu

[tool.pylint.master]
init-import = "yes"
load-plugins = ["pylint.extensions.bad_builtin", "pylint.extensions.mccabe"]
load-plugins = ["pylint.extensions.bad_builtin", "pylint.extensions.mccabe", "pylint_pytest"]
extension-pkg-allow-list = ["lxml.etree"]

[tool.pylint.similarities]
Expand Down
11 changes: 9 additions & 2 deletions backend/tests/cli/test_workspace_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ def test_restore_workspace_without_sidecar(
pvc_created = False

def mock_create_namespaced_persistent_volume_claim(
self, namespace: str, pvc: kubernetes.client.V1PersistentVolumeClaim
# pylint: disable=unused-argument
self,
namespace: str,
pvc: kubernetes.client.V1PersistentVolumeClaim,
):
assert namespace == "default"
assert pvc.metadata.name == "my-volume-name"
Expand Down Expand Up @@ -189,7 +192,10 @@ def test_restore_workspace_with_sidecar(
)

def mock_create_namespaced_persistent_volume_claim(
self, namespace: str, pvc: kubernetes.client.V1PersistentVolumeClaim
# pylint: disable=unused-argument
self,
namespace: str,
pvc: kubernetes.client.V1PersistentVolumeClaim,
):
assert namespace == "default"
assert pvc.metadata.name == "my-volume-name"
Expand Down Expand Up @@ -237,6 +243,7 @@ def is_open(self):
def close(self):
self._connected = False

# pylint: disable=unused-argument
def recv_data_frame(self, wait):
if self._blocks:
return ABNF.OPCODE_BINARY, Frame(self._blocks.pop(0))
Expand Down
1 change: 1 addition & 0 deletions backend/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def mock_get_db() -> orm.Session:

app.dependency_overrides[database.get_db] = mock_get_db

# pylint: disable=unused-argument
def commit(*args, **kwargs):
session.flush()
session.expire_all()
Expand Down
5 changes: 2 additions & 3 deletions backend/tests/core/test_auth_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

from capellacollab.config import config
from capellacollab.core.authentication import api_key_cookie, exceptions, oidc
from capellacollab.core.authentication import routes
from capellacollab.core.authentication import routes as auth_routes
from capellacollab.users import crud as users_crud
from capellacollab.users import models as users_models
Expand Down Expand Up @@ -216,7 +215,7 @@ def test_validate_id_token_nonce_mismatch(
monkeypatch.setattr(api_key_cookie, "JWTAPIKeyCookie", mock_jwt_api_cookie)

with pytest.raises(exceptions.NonceMismatchError):
routes.validate_id_token("any", "correct-nonce")
auth_routes.validate_id_token("any", "correct-nonce")


@responses.activate
Expand All @@ -233,4 +232,4 @@ def test_validate_id_token_audience_mismatch(
monkeypatch.setattr(api_key_cookie, "JWTAPIKeyCookie", mock_jwt_api_cookie)

with pytest.raises(exceptions.UnauthenticatedError):
routes.validate_id_token("any", "mock-nonce")
auth_routes.validate_id_token("any", "mock-nonce")
1 change: 0 additions & 1 deletion backend/tests/projects/test_projects_users_routes.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# SPDX-FileCopyrightText: Copyright DB InfraGO AG and contributors
# SPDX-License-Identifier: Apache-2.0

import pytest
from fastapi import testclient
from sqlalchemy import orm

Expand Down
16 changes: 14 additions & 2 deletions backend/tests/projects/toolmodels/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,30 @@ def fixture_mock_git_valkey_cache(monkeypatch: pytest.MonkeyPatch):
class MockGitValkeyCache:
cache: dict[str, tuple[datetime.datetime, bytes]] = {}

# pylint: disable=unused-argument
def __init__(self, *args, **kwargs) -> None:
super().__init__()

def get_file_data(
self, file_path: str, revision: str, logger: logging.LoggerAdapter
# pylint: disable=unused-argument
self,
file_path: str,
revision: str,
logger: logging.LoggerAdapter,
) -> tuple[datetime.datetime, bytes] | None:
return MockGitValkeyCache.cache.get(f"f:{file_path}", None)

def get_artifact_data(
self, job_id: str, file_path: str, logger: logging.LoggerAdapter
# pylint: disable=unused-argument
self,
job_id: str,
file_path: str,
logger: logging.LoggerAdapter,
) -> tuple[datetime.datetime, bytes] | None:
return MockGitValkeyCache.cache.get(f"a:{file_path}:{job_id}")

def put_file_data(
# pylint: disable=unused-argument
self,
file_path: str,
last_updated: datetime.datetime,
Expand All @@ -48,6 +58,7 @@ def put_file_data(
)

def put_artifact_data(
# pylint: disable=unused-argument
self,
job_id: str,
file_path: str,
Expand Down Expand Up @@ -257,6 +268,7 @@ def fixture_mock_git_get_commit_information_api(
@pytest.fixture(name="mock_add_user_to_t4c_repository")
def fixture_mock_add_user_to_t4c_repository(monkeypatch: pytest.MonkeyPatch):
def mock_add_user_to_repository(
# pylint: disable=unused-argument
instance: t4c_models.DatabaseT4CInstance,
repository_name: str,
username: str,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@
from capellacollab.projects.toolmodels.backups.runs import (
injectables as runs_injectables,
)
from capellacollab.projects.toolmodels.backups.runs import (
models as runs_models,
)
from capellacollab.users import crud as users_crud
from capellacollab.users import models as users_models

Expand All @@ -35,7 +32,10 @@ def fixture_unix_time_in_ns() -> int:
@pytest.fixture(name="patch_loki")
def fixture_patch_loki(monkeypatch: pytest.MonkeyPatch, unix_time_in_ns: int):
def fetch_logs_from_loki(
query, start_time: datetime.datetime, end_time: datetime.datetime
# pylint: disable=unused-argument
query,
start_time: datetime.datetime,
end_time: datetime.datetime,
):
return [
{
Expand Down Expand Up @@ -174,7 +174,9 @@ def test_get_logs_with_loki_disabled(

@pytest.fixture(name="mock_pipeline_run")
def fixture_mock_pipeline_run():
mock_pipeline_run = mock.MagicMock(spec=runs_models.DatabasePipelineRun)
mock_pipeline_run = mock.MagicMock(
spec=pipeline_runs_models.DatabasePipelineRun
)

# Assign the values you want the mock object to return
mock_pipeline_run.id = "mock_id"
Expand All @@ -197,7 +199,9 @@ def fixture_mock_pipeline_run():
def fixture_override_get_existing_pipeline_run_dependency(
mock_pipeline_run: mock.Mock,
):
def get_mock_existing_pipeline_run() -> runs_models.DatabasePipelineRun:
def get_mock_existing_pipeline_run() -> (
pipeline_runs_models.DatabasePipelineRun
):
return mock_pipeline_run

app.dependency_overrides[runs_injectables.get_existing_pipeline_run] = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class MockOperator:
cronjob_counter = 0

def create_cronjob(
# pylint: disable=unused-argument
self,
*args,
**kwargs,
Expand All @@ -38,7 +39,6 @@ def _generate_id(self) -> str:

def delete_cronjob(self, _id: str):
self.cronjob_counter -= 1
return


@pytest.fixture(name="mockoperator")
Expand Down Expand Up @@ -179,6 +179,7 @@ def test_delete_pipeline(
run_nightly: bool,
):
def mock_remove_user_from_repository(
# pylint: disable=unused-argument
instance: t4c_models.DatabaseT4CInstance,
repository_name: str,
username: str,
Expand Down
5 changes: 2 additions & 3 deletions backend/tests/projects/toolmodels/test_jupyter_fileshare.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,9 @@

import pytest
from fastapi import testclient
from sqlalchemy import orm

import capellacollab.sessions.operators
from capellacollab.core.database import migration as database_migration
from capellacollab.projects import models as projects_models
from capellacollab.projects.toolmodels import crud as toolmodels_crud
from capellacollab.projects.toolmodels import models as toolmodels_models
from capellacollab.tools import models as tools_models

Expand All @@ -18,13 +15,15 @@ class MockOperator:
_deleted_volumes = 0

def create_persistent_volume(
# pylint: disable=unused-argument
self,
name: str,
size: str,
labels: dict[str, str] = None,
):
self._created_volumes += 1

# pylint: disable=unused-argument
def delete_persistent_volume(self, name: str):
self._deleted_volumes += 1

Expand Down
2 changes: 1 addition & 1 deletion backend/tests/projects/toolmodels/test_model_badge.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def fixture_mock_get_model_badge_from_artifacts_api(
)
case git_models.GitType.GITHUB:
responses.get(
f"https://example.com/api/v4/repos/test/project/actions/artifacts/12347/zip",
"https://example.com/api/v4/repos/test/project/actions/artifacts/12347/zip",
status=200,
body=get_zipfile(),
content_type="application/zip",
Expand Down
2 changes: 0 additions & 2 deletions backend/tests/projects/users/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@
import pytest
from sqlalchemy import orm

import capellacollab.projects.models as projects_models
import capellacollab.projects.users.crud as projects_users_crud
import capellacollab.projects.users.models as projects_users_models
import capellacollab.users.models as users_models
from capellacollab.projects import models as projects_models
from capellacollab.users import models as users_models

Expand Down
3 changes: 0 additions & 3 deletions backend/tests/sessions/hooks/test_http_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@

import logging

import pytest

from capellacollab.sessions import auth as sessions_auth
from capellacollab.sessions import models as sessions_models
from capellacollab.sessions.hooks import http
from capellacollab.sessions.hooks import interface as sessions_hooks_interface
Expand Down
2 changes: 2 additions & 0 deletions backend/tests/sessions/hooks/test_jupyter_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ def test_jupyter_successful_volume_mount(
db: orm.Session,
):
class MockOperator:
# pylint: disable=unused-argument
def persistent_volume_exists(self, name: str) -> bool:
return True

Expand All @@ -41,6 +42,7 @@ def test_jupyter_volume_mount_not_found(
db: orm.Session,
):
class MockOperator:
# pylint: disable=unused-argument
def persistent_volume_exists(self, name: str) -> bool:
return False

Expand Down
2 changes: 2 additions & 0 deletions backend/tests/sessions/hooks/test_networking_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def test_network_policy_created(
network_policy_counter = 0

def mock_create_namespaced_network_policy(
# pylint: disable=unused-argument
self,
namespace: str,
network_policy: kubernetes.client.V1PersistentVolumeClaim,
Expand Down Expand Up @@ -45,6 +46,7 @@ def test_network_policy_deleted(
network_policy_del_counter = 0

def mock_delete_namespaced_network_policy(
# pylint: disable=unused-argument
self,
name: str,
namespace: str,
Expand Down
Loading

0 comments on commit 254d430

Please sign in to comment.