Merge pull request #1123 from CodeForPhilly/staging

Weekly PR from Staging to Main

Author: nlebovits

.github/CODEOWNERS

@@ -1 +1 @@
-@nlebovits @brandonfcohen1 @marvieqa @bacitracin @paulhchoi @CodeWritingCow
+@nlebovits @brandonfcohen1 @marvieqa @bacitracin @paulhchoi @CodeWritingCow @SofiaFasullo @cfreedman

.github/workflows/pr_checks_backend.yml

@@ -29,6 +29,53 @@ jobs:
         with:
           python-version: '3.11.4'
 
+  test:
+    runs-on: ubuntu-latest
+    needs: setup
+    defaults:
+      run:
+        working-directory: data/src
+    env:
+      VACANT_LOTS_DB: 'postgresql://postgres:${{ secrets.POSTGRES_PASSWORD }}@localhost:5433/vacantlotdb'
+    services:
+      postgres:
+        image: postgis/postgis:16-3.4
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: ${{ secrets.POSTGRES_PASSWORD }}
+          POSTGRES_DB: vacantlotdb
+        ports:
+          - 5433:5432
+        # Set health checks to wait until postgres is ready
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11.4'
+
+      - name: Install and configure pipenv
+        run: |
+          python -m pip install --upgrade pip
+          pip install pipenv
+          echo "Using Python: $(which python)"
+          pipenv --python $(which python) install --dev
+
+      - name: Install awkde
+        working-directory: data/src/awkde
+        run: pipenv run pip install .
+
+      - name: Run Pytest
+        working-directory: data/src
+        run: PYTHONPATH=$PYTHONPATH:. pipenv run pytest
+
   run-formatter:
     runs-on: ubuntu-latest
     needs: setup

data/src/classes/backup_archive_database.py

@@ -60,17 +60,23 @@ def backup_schema(self):
             + backup_schema_name
             + ".spatial_ref_sys/public.spatial_ref_sys/' | psql -v ON_ERROR_STOP=1 "
             + url
-            + " > /dev/null "
         )
         log.debug(mask_password(pgdump_command))
-        complete_process = subprocess.run(pgdump_command, check=False, shell=True)
+        complete_process = subprocess.run(
+            pgdump_command,
+            check=False,
+            shell=True,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            text=True,
+        )
 
         if complete_process.returncode != 0 or complete_process.stderr:
             raise RuntimeError(
                 "pg_dump command "
                 + mask_password(pgdump_command)
                 + " did not exit with success. "
-                + complete_process.stderr.decode()
+                + complete_process.stderr
             )
 
     def archive_backup_schema(self):
@@ -123,4 +129,4 @@ def backup_tiles_file(self):
             bucket.copy_blob(blob,destination_bucket=bucket,new_name=backup_file_name)
             count += 1
         if count == 0:
-            log.warning("No files were found to back up.")
+            log.warning("No files were found to back up.")

data/src/classes/featurelayer.py

@@ -42,7 +42,6 @@ def google_cloud_bucket() -> Bucket:
     return storage_client.bucket(bucket_name)
 
 
-bucket = google_cloud_bucket()
 
 
 class FeatureLayer:
@@ -61,6 +60,7 @@ def __init__(
         from_xy=False,
         use_wkb_geom_field=None,
         cols: list[str] = None,
+        bucket: Bucket = None
     ):
         self.name = name
         self.esri_rest_urls = (
@@ -77,6 +77,7 @@ def __init__(
         self.psql_table = name.lower().replace(" ", "_")
         self.input_crs = "EPSG:4326" if not from_xy else USE_CRS
         self.use_wkb_geom_field = use_wkb_geom_field
+        self.bucket = bucket or google_cloud_bucket()
 
         inputs = [self.esri_rest_urls, self.carto_sql_queries, self.gdf]
         non_none_inputs = [i for i in inputs if i is not None]
@@ -331,7 +332,7 @@ def build_and_publish(self, tiles_file_id_prefix: str) -> None:
         df_no_geom.to_parquet(temp_parquet)
 
         # Upload Parquet to Google Cloud Storage
-        blob_parquet = bucket.blob(f"{tiles_file_id_prefix}.parquet")
+        blob_parquet = self.bucket.blob(f"{tiles_file_id_prefix}.parquet")
         try:
             blob_parquet.upload_from_filename(temp_parquet)
             parquet_size = os.stat(temp_parquet).st_size
@@ -400,7 +401,7 @@ def build_and_publish(self, tiles_file_id_prefix: str) -> None:
 
         # Upload PMTiles to Google Cloud Storage
         for file in write_files:
-            blob = bucket.blob(file)
+            blob = self.bucket.blob(file)
             try:
                 blob.upload_from_filename(temp_merged_pmtiles)
                 print(f"PMTiles upload successful for {file}!")

data/src/test/conftest.py

@@ -0,0 +1,17 @@
+from unittest.mock import MagicMock
+
+import pytest
+from google.cloud.storage import Bucket
+
+
+@pytest.fixture(autouse=True)
+def mock_gcp_bucket(monkeypatch):
+    mock_bucket = MagicMock(spec=Bucket)
+
+    monkeypatch.setattr("classes.featurelayer.google_cloud_bucket", lambda: mock_bucket)
+
+    return mock_bucket
+
+
+# Tell vulture this is used:
+_ = mock_gcp_bucket  # Used indirectly by pytest

data/src/test/test_data_utils.py

@@ -1,21 +1,63 @@
 import unittest
 import zipfile
 from io import BytesIO
-from unittest.mock import MagicMock, patch
+from unittest.mock import MagicMock, Mock, patch
 
 import geopandas as gpd
-from config.config import USE_CRS
 from data_utils.park_priority import get_latest_shapefile_url, park_priority
 from data_utils.ppr_properties import ppr_properties
 from data_utils.vacant_properties import vacant_properties
 from shapely.geometry import Point
 
+from config.config import USE_CRS
+
 
 class TestDataUtils(unittest.TestCase):
     """
     Test methods for data utils feature layer classes
     """
 
+    @classmethod
+    def setUpClass(cls):
+        # Create the mock GeoDataFrame that will be reused
+        cls.mock_gdf = gpd.GeoDataFrame(
+            {
+                "ADDRESS": ["123 Main St"],
+                "OWNER1": ["John Doe"],
+                "OWNER2": ["Jane Doe"],
+                "BLDG_DESC": ["House"],
+                "CouncilDistrict": [1],
+                "ZoningBaseDistrict": ["R1"],
+                "ZipCode": ["19107"],
+                "OPA_ID": ["12345"],
+                "geometry": [Point(-75.1652, 39.9526)],
+            },
+            crs="EPSG:4326",
+        )
+
+    def setUp(self):
+        # Set up the mocks that will be used in each test
+        self.patcher1 = patch("data_utils.vacant_properties.google_cloud_bucket")
+        self.patcher2 = patch("geopandas.read_file")
+
+        self.mock_gcs = self.patcher1.start()
+        self.mock_gpd = self.patcher2.start()
+
+        # Set up the mock chain
+        mock_blob = Mock()
+        mock_blob.exists.return_value = True
+        mock_blob.download_as_bytes.return_value = b"dummy bytes"
+
+        mock_bucket = Mock()
+        mock_bucket.blob.return_value = mock_blob
+
+        self.mock_gcs.return_value = mock_bucket
+        self.mock_gpd.return_value = self.mock_gdf
+
+    def tearDown(self):
+        self.patcher1.stop()
+        self.patcher2.stop()
+
     def test_get_latest_shapefile_url(self):
         """
         Test the get_latest_shapefile_url function.
@@ -49,7 +91,7 @@ def test_get_latest_shapefile_url_mock(self, mock_get):
     def test_park_priority(
         self,
         mock_extract,
-        mock_makedirs,
+        _mock_makedirs,
         mock_exists,
         mock_to_file,
         mock_read_file,

data/src/test/test_diff_backup.py

@@ -1,5 +1,7 @@
+import os
 from datetime import datetime
 
+import pytest
 from classes.backup_archive_database import (
     BackupArchiveDatabase,
     backup_schema_name,
@@ -10,6 +12,10 @@
 from config.psql import conn, local_engine
 from sqlalchemy import inspect
 
+pytestmark = pytest.mark.skip(
+    reason="Skipping tests. The tests in test_diff_backup are designed for stateful, manual testing."
+)
+
 
 class TestDiffBackup:
     """
@@ -60,26 +66,38 @@ def test_detail_report(self):
         url = diff.detail_report("vacant_properties")
         print(url)
 
+    @pytest.mark.skipif(
+        not os.getenv("INTEGRATION_TESTING"),
+        reason="For manual integration testing only. Export INTEGRATION_TESTING=True to run",
+    )
     def test_upload_to_gcp(self):
         """test a simple upload to Google cloud"""
         bucket = google_cloud_bucket()
         blob = bucket.blob("test.txt")
         blob.upload_from_string("test")
 
+    @pytest.mark.skipif(
+        not os.getenv("INTEGRATION_TESTING"),
+        reason="For manual integration testing only. Export INTEGRATION_TESTING=True to run",
+    )
     def test_send_report_to_slack(self):
         """CAREFUL: if configured, this will send a message to Slack, potentially our prod channel"""
         diff = DiffReport()
         diff.report = "This is the report"
         diff.send_report_to_slack()
 
+    @pytest.mark.skipif(
+        not os.getenv("INTEGRATION_TESTING"),
+        reason="For manual integration testing only. Export INTEGRATION_TESTING=True to run",
+    )
     def test_email_report(self):
         """CAREFUL: if configured, this will send email if configured"""
         diff = DiffReport()
         diff.report = "This is the report"
         diff.email_report()
 
     def test_is_backup_schema_exists(self):
-        """test method for whether the backup schema exists """    
+        """test method for whether the backup schema exists"""
         if TestDiffBackup.backup.is_backup_schema_exists():
             TestDiffBackup.backup.archive_backup_schema()
             conn.commit()
@@ -91,8 +109,6 @@ def test_is_backup_schema_exists(self):
             conn.commit()
             assert not TestDiffBackup.backup.is_backup_schema_exists()
 
-    
     def test_backup_tiles_file(self):
-        """ test backing up the tiles file """
+        """test backing up the tiles file"""
         TestDiffBackup.backup.backup_tiles_file()
-

data/src/test/test_slack_error_reporter.py

@@ -1,9 +1,8 @@
+import os
+import sys
 import unittest
 from unittest.mock import patch
 
-import sys
-import os
-
 sys.path.append(os.path.dirname(os.path.abspath(__file__)) + "/..")
 
 from classes.slack_error_reporter import (
@@ -18,7 +17,7 @@ class TestSlackNotifier(unittest.TestCase):
     @patch(
         "classes.slack_error_reporter.os.getenv", return_value="mock_slack_token"
     )  # Correct patching
-    def test_send_error_to_slack(self, mock_getenv, mock_slack_post):
+    def test_send_error_to_slack(self, _mock_getenv, mock_slack_post):
         """Test that Slack error reporting is triggered correctly."""
 
         error_message = "Test error message"
@@ -39,7 +38,7 @@ def test_send_error_to_slack(self, mock_getenv, mock_slack_post):
     @patch(
         "classes.slack_error_reporter.os.getenv", return_value=None
     )  # Simulate missing Slack token
-    def test_no_error_no_slack_message(self, mock_getenv, mock_slack_post):
+    def test_no_error_no_slack_message(self, _mock_getenv, mock_slack_post):
         """Test that Slack notification is not triggered if there's no error."""
 
         # Call the Slack notification function (with no valid token)