chore: Enable pylint in tests folder

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

.github/workflows/lint.yml

@@ -50,7 +50,7 @@ jobs:
           python -m pip install pre-commit
       - name: Install pylint
         run: |-
-          python -m pip install pylint
+          cd backend/ && python -m pip install '.[dev]' && cd ..
       - name: Run pre-commit
         run: |-
           pre-commit run --all-files

.pre-commit-config.yaml

@@ -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:

backend/capellacollab/core/database/decorator.py

@@ -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

backend/capellacollab/core/database/migration.py

@@ -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",
@@ -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:
@@ -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)

backend/capellacollab/sessions/crud.py

@@ -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

backend/pyproject.toml

@@ -59,6 +59,7 @@ dev = [
   "isort",
   "mypy",
   "pylint",
+  "pylint-pytest",
   "pytest",
   "testcontainers",
   "httpx",
@@ -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]

backend/tests/cli/test_workspace_backup.py

@@ -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"
@@ -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"
@@ -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))

backend/tests/conftest.py

@@ -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()

backend/tests/core/test_auth_routes.py

@@ -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
@@ -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
@@ -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")

backend/tests/projects/test_projects_users_routes.py

@@ -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
 

backend/tests/projects/toolmodels/conftest.py

@@ -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,
@@ -48,6 +58,7 @@ def put_file_data(
             )
 
         def put_artifact_data(
+            # pylint: disable=unused-argument
             self,
             job_id: str,
             file_path: str,
@@ -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,

backend/tests/projects/toolmodels/pipelines/pipeline-runs/test_pipeline_runs.py

@@ -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
 
@@ -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 [
             {
@@ -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"
@@ -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] = (

backend/tests/projects/toolmodels/pipelines/test_pipelines.py

@@ -26,6 +26,7 @@ class MockOperator:
     cronjob_counter = 0
 
     def create_cronjob(
+        # pylint: disable=unused-argument
         self,
         *args,
         **kwargs,
@@ -38,7 +39,6 @@ def _generate_id(self) -> str:
 
     def delete_cronjob(self, _id: str):
         self.cronjob_counter -= 1
-        return
 
 
 @pytest.fixture(name="mockoperator")
@@ -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,

backend/tests/projects/toolmodels/test_jupyter_fileshare.py

@@ -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
 
@@ -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
 

backend/tests/projects/toolmodels/test_model_badge.py

@@ -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",

backend/tests/projects/users/fixtures.py

@@ -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
 

backend/tests/sessions/hooks/test_http_hook.py

@@ -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

backend/tests/sessions/hooks/test_jupyter_hook.py

@@ -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
 
@@ -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
 

backend/tests/sessions/hooks/test_networking_hook.py

@@ -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,
@@ -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,

backend/tests/sessions/hooks/test_persistent_workspace.py

@@ -62,7 +62,10 @@ def test_workspace_is_created(
     volume_name = None
 
     def mock_create_namespaced_persistent_volume_claim(
-        self, ns: str, pvc: kubernetes.client.V1PersistentVolumeClaim
+        # pylint: disable=unused-argument
+        self,
+        ns: str,
+        pvc: kubernetes.client.V1PersistentVolumeClaim,
     ):
         nonlocal created_volumes, volume_name
         created_volumes += 1
@@ -102,6 +105,7 @@ def test_existing_workspace_is_mounted(
     created_volumes = 0
     volume_name = None
 
+    # pylint: disable=unused-argument
     def mock_create_namespaced_persistent_volume_claim(self, ns, pvc):
         nonlocal created_volumes, volume_name
         created_volumes += 1