test: unit tests for collector (#8)

* test: unit tests for collector

* CI: add python tests

.github/workflows/pr.yaml

@@ -0,0 +1,27 @@
+name: PR checks
+on:
+  pull_request:
+
+jobs:
+   test-python:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: dorny/paths-filter@v3
+        id: filter
+        with:
+          filters: |
+            py:
+              - '**/*.py'
+
+      - name: Install Python dependencies
+        if: ${{ steps.filter.outputs.py == 'true' }}
+        run: |
+          pip3 install -r requirements.txt
+          pip3 install pytest
+
+      - name: Run Python tests 
+        if: ${{ steps.filter.outputs.py == 'true' }}
+        run: |
+          pytest 

collector/extra_fields.py

@@ -147,15 +147,16 @@ def fetch_eligible_snaps(db_session: Session) -> List[Snap]:
         return snaps
     except Exception as e:
         logger.error(f"Error querying eligible snaps: {e}")
+        raise
 
 
 def update_snap_metrics():
-    with Session(bind=db.engine) as db_session:
-        try:
-            eligible_snaps = fetch_eligible_snaps(db_session)
-            fetch_and_update_metrics_for_snaps(eligible_snaps, db_session)
-        except Exception as e:
-            logger.error(f"Error during metrics update process: {e}")
+    try:
+        eligible_snaps = fetch_eligible_snaps(db.session)
+        fetch_and_update_metrics_for_snaps(eligible_snaps, db.session)
+    except Exception as e:
+        logger.error(f"Error during metrics update process: {e}")
+        raise
 
 
 def fetch_extra_fields():

pyproject.toml

@@ -0,0 +1,5 @@
+[tool.pytest.ini_options]
+testpaths = ["tests"]
+pythonpath = [
+  "."
+]

snaprecommend/api.py

@@ -36,7 +36,13 @@ def get_top_snaps_by_field(
     field = getattr(Scores, field)
     order = field.asc() if ascending else field.desc()
 
-    snaps = db.session.query(Snap).join(Scores).order_by(order).limit(limit)
+    snaps = (
+        db.session.query(Snap)
+        .filter(Snap.reaches_min_threshold.is_(True))
+        .join(Scores)
+        .order_by(order)
+        .limit(limit)
+    )
 
     return list(snaps)
 

tests/test_collect.py

@@ -0,0 +1,152 @@
+import pytest
+import requests
+from datetime import datetime
+from unittest.mock import MagicMock, patch
+from collector.collect import (
+    get_snap_page,
+    upsert_snap,
+    bulk_upsert_snaps,
+    collect_initial_snap_data,
+)
+from sqlalchemy.orm import Session
+
+
+@pytest.fixture
+def mock_session():
+    """Fixture to create a mocked SQLAlchemy session."""
+    return MagicMock(spec=Session)
+
+
+@pytest.fixture
+def sample_snap():
+    """Fixture to provide a sample snap."""
+    return {
+        "snap_id": "snap1",
+        "package_name": "test-snap",
+        "last_updated": "2024-12-16 09:32:04.767",
+        "summary": "Test summary",
+        "description": "Test description",
+        "title": "Test Snap",
+        "version": "1.0",
+        "publisher": "Canonical",
+        "revision": 1,
+        "links": {
+            "website": ["https://example.com"],
+            "contact": ["[email protected]"],
+        },
+        "media": [{"type": "icon", "url": "https://example.com/icon.png"}],
+        "developer_validation": True,
+        "license": "MIT",
+    }
+
+
+@patch("collector.collect.requests.get")
+def test_get_snap_page(mock_get):
+    """Test fetching a page of snaps."""
+    mock_response = MagicMock()
+    mock_response.json.return_value = {
+        "_embedded": {
+            "clickindex:package": [
+                {"snap_id": "snap1", "links": {}, "media": []},
+                {"snap_id": "snap2", "links": {}, "media": []},
+            ]
+        },
+        "_links": {"next": {"href": "next_page"}},
+    }
+    mock_get.return_value = mock_response
+
+    snaps, has_next = get_snap_page(1)
+
+    assert len(snaps) == 2
+    assert has_next is True
+    mock_get.assert_called_once_with(
+        "http://api.snapcraft.io/api/v1/snaps/search?fields=snap_id,package_name,last_updated,summary,description,title,version,publisher,revision,links,media,developer_validation,license&page=1"
+    )
+
+
+@patch("collector.collect.requests.get")
+def test_no_next_page(mock_get):
+    mock_response = {"_embedded": {"clickindex:package": []}, "_links": {}}
+    mock_get.return_value.status_code = 200
+    mock_get.return_value.json.return_value = mock_response
+
+    snaps, has_next = get_snap_page(1)
+
+    assert snaps == []
+    assert has_next is False
+
+
+@patch("collector.collect.requests.get")
+def test_api_error(mock_get):
+    mock_get.return_value.status_code = 500
+    mock_get.side_effect = requests.exceptions.HTTPError(
+        "Internal Server Error"
+    )
+
+    with pytest.raises(requests.exceptions.HTTPError):
+        get_snap_page(1)
+
+
+def test_upsert_snap(mock_session, sample_snap):
+    """Test upserting a single snap."""
+    upsert_snap(mock_session, sample_snap)
+
+    mock_session.merge.assert_called_once()
+    snap_instance = mock_session.merge.call_args[0][0]
+    assert snap_instance.snap_id == sample_snap["snap_id"]
+    assert snap_instance.name == sample_snap["package_name"]
+    assert snap_instance.website == sample_snap["links"]["website"][0]
+    assert snap_instance.contact == sample_snap["links"]["contact"][0]
+
+
+@patch("collector.collect.insert")
+def test_bulk_upsert_snaps(mock_insert, mock_session, sample_snap):
+    """Test bulk upsert of snaps."""
+    mock_stmt = MagicMock()
+    mock_insert.return_value.values.return_value.on_conflict_do_update.return_value = (
+        mock_stmt
+    )
+
+    bulk_upsert_snaps(mock_session, [sample_snap])
+
+    mock_insert.assert_called_once()
+    mock_insert.return_value.values.assert_called_once_with(
+        [
+            {
+                "snap_id": sample_snap["snap_id"],
+                "name": sample_snap["package_name"],
+                "icon": "https://example.com/icon.png",
+                "summary": sample_snap["summary"],
+                "description": sample_snap["description"],
+                "title": sample_snap["title"],
+                "website": "https://example.com",
+                "version": sample_snap["version"],
+                "publisher": sample_snap["publisher"],
+                "revision": sample_snap["revision"],
+                "contact": "[email protected]",
+                "links": sample_snap["links"],
+                "media": sample_snap["media"],
+                "developer_validation": sample_snap["developer_validation"],
+                "license": sample_snap["license"],
+                "last_updated": datetime.fromisoformat(
+                    sample_snap["last_updated"]
+                ),
+            }
+        ]
+    )
+    mock_session.execute.assert_called_once_with(mock_stmt)
+
+
+@patch("collector.collect.insert_snaps", return_value=5)
+@patch("collector.collect.logger")
+def test_collect_initial_snap_data(mock_logger, mock_insert_snaps):
+    """Test the entire snap collection process."""
+    collect_initial_snap_data()
+
+    mock_insert_snaps.assert_called_once()
+    mock_logger.info.assert_any_call(
+        "Starting the snap data ingestion process."
+    )
+    mock_logger.info.assert_any_call(
+        "Snap data ingestion process completed. 5 snaps inserted."
+    )

tests/test_extra_fields.py

@@ -0,0 +1,117 @@
+import pytest
+from unittest.mock import MagicMock, patch
+from collector.extra_fields import (
+    calculate_latest_active_devices,
+    fetch_metrics_from_api,
+    process_and_update_snap_metrics,
+    fetch_and_update_metrics_for_snaps,
+    get_metrics_time_range,
+    fetch_eligible_snaps,
+    update_snap_metrics,
+    fetch_extra_fields,
+)
+from snaprecommend.models import Snap
+from sqlalchemy.orm import Session
+from datetime import datetime, timedelta
+
+
+@pytest.fixture
+def mock_session():
+    return MagicMock(spec=Session)
+
+
+@pytest.fixture
+def sample_snap():
+    return Snap(
+        snap_id="snap1",
+        name="test-snap",
+        active_devices=0,
+        reaches_min_threshold=True,
+    )
+
+
+def test_calculate_latest_active_devices():
+    metrics_data = {
+        "series": [
+            {"values": [0, 10, 20, 30]},
+            {"values": [5, 15, 25, 35]},
+        ]
+    }
+    result = calculate_latest_active_devices(metrics_data)
+    assert result == 65
+
+
+@patch("collector.extra_fields.requests.post")
+@patch("collector.extra_fields.get_auth_header")
+def test_fetch_metrics_from_api(mock_get_auth_header, mock_post, sample_snap):
+    mock_get_auth_header.return_value = "Bearer token"
+    mock_response = MagicMock()
+    mock_response.json.return_value = {"metrics": []}
+    mock_post.return_value = mock_response
+
+    result = fetch_metrics_from_api([sample_snap], "2023-01-01", "2023-01-31")
+    assert result == {"metrics": []}
+    mock_post.assert_called_once()
+
+
+def test_process_and_update_snap_metrics(mock_session, sample_snap):
+    metrics_data = {
+        "metrics": [
+            {"series": [{"values": [0, 10, 20, 30]}]},
+        ]
+    }
+    process_and_update_snap_metrics([sample_snap], metrics_data, mock_session)
+    assert sample_snap.active_devices == 30
+    mock_session.commit.assert_called_once()
+
+
+@patch("collector.extra_fields.fetch_metrics_from_api")
+@patch("collector.extra_fields.process_and_update_snap_metrics")
+@patch("collector.extra_fields.get_metrics_time_range")
+def test_fetch_and_update_metrics_for_snaps(
+    mock_get_metrics_time_range,
+    mock_process_and_update_snap_metrics,
+    mock_fetch_metrics_from_api,
+    mock_session,
+    sample_snap,
+):
+    mock_get_metrics_time_range.return_value = ("2023-01-01", "2023-01-31")
+    mock_fetch_metrics_from_api.return_value = {"metrics": []}
+
+    fetch_and_update_metrics_for_snaps([sample_snap], mock_session)
+    mock_fetch_metrics_from_api.assert_called_once()
+    mock_process_and_update_snap_metrics.assert_called_once()
+
+
+def test_get_metrics_time_range():
+    start_date, end_date = get_metrics_time_range()
+    assert end_date == datetime.now().strftime("%Y-%m-%d")
+    assert start_date == (datetime.now() - timedelta(days=30)).strftime(
+        "%Y-%m-%d"
+    )
+
+
+def test_fetch_eligible_snaps(mock_session):
+    mock_session.query().filter().all.return_value = [sample_snap]
+    result = fetch_eligible_snaps(mock_session)
+    assert result == [sample_snap]
+    mock_session.query().filter().all.assert_called_once()
+
+
+@patch("collector.extra_fields.fetch_eligible_snaps")
+@patch("collector.extra_fields.fetch_and_update_metrics_for_snaps")
+def test_update_snap_metrics(
+    mock_fetch_and_update_metrics_for_snaps,
+    mock_fetch_eligible_snaps,
+):
+    mock_fetch_eligible_snaps.return_value = [sample_snap]
+
+    update_snap_metrics()
+    mock_fetch_eligible_snaps.assert_called_once()
+    mock_fetch_and_update_metrics_for_snaps.assert_called_once()
+
+
+@patch("collector.extra_fields.update_snap_metrics")
+def test_fetch_extra_fields(mock_update_snap_metrics):
+    fetch_extra_fields()
+    mock_update_snap_metrics.assert_called_once()