Add Tests for PR#790

.github/workflows/ci.yml

@@ -22,11 +22,11 @@ jobs:
   lint:
     name: Run Lint
 
-    uses: codecov/gha-workflows/.github/workflows/lint.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/lint.yml@v1.2.24
 
   build:
     name: Build Worker
-    uses: codecov/gha-workflows/.github/workflows/build-app.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/build-app.yml@v1.2.24
 
     secrets: inherit
     with:
@@ -35,14 +35,14 @@ jobs:
   codecovstartup:
     name: Codecov Startup
     needs: build
-    uses: codecov/gha-workflows/.github/workflows/codecov-startup.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/codecov-startup.yml@v1.2.24
 
     secrets: inherit
 
   test:
     name: Test
     needs: [build]
-    uses: codecov/gha-workflows/.github/workflows/run-tests.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/run-tests.yml@v1.2.24
 
     secrets: inherit
     with:
@@ -52,7 +52,7 @@ jobs:
     name: Build Self Hosted Worker
     needs: [build, test]
 
-    uses: codecov/gha-workflows/.github/workflows/self-hosted.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/self-hosted.yml@v1.2.24
     secrets: inherit
     with:
       repo: ${{ vars.CODECOV_IMAGE_V2 || 'codecov/self-hosted-worker' }}
@@ -61,7 +61,7 @@ jobs:
     name: Push Staging Image
     needs: [build, test]
     if: ${{ github.event_name == 'push' && (github.event.ref == 'refs/heads/main' || github.event.ref == 'refs/heads/staging') && github.repository_owner == 'codecov' }}
-    uses: codecov/gha-workflows/.github/workflows/push-env.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/push-env.yml@v1.2.24
     secrets: inherit
     with:
       environment: staging
@@ -71,7 +71,7 @@ jobs:
     name: Push Production Image
     needs: [build, test]
     if: ${{ github.event_name == 'push' && github.event.ref == 'refs/heads/main' && github.repository_owner == 'codecov' }}
-    uses: codecov/gha-workflows/.github/workflows/push-env.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/push-env.yml@v1.2.24
     secrets: inherit
     with:
       environment: production
@@ -82,7 +82,7 @@ jobs:
     needs: [build-self-hosted, test]
     secrets: inherit
     if: ${{ github.event_name == 'push' && github.event.ref == 'refs/heads/main' && github.repository_owner == 'codecov' }}
-    uses: codecov/gha-workflows/.github/workflows/self-hosted.yml@v1.2.23
+    uses: codecov/gha-workflows/.github/workflows/self-hosted.yml@v1.2.24
     with:
       push_rolling: true
       repo: ${{ vars.CODECOV_IMAGE_V2 || 'codecov/self-hosted-worker' }}

helpers/checkpoint_logger/tests/test_prometheus_handler.py

@@ -0,0 +1,84 @@
+import unittest
+from unittest.mock import patch, MagicMock
+from helpers.checkpoint_logger.prometheus import PrometheusCheckpointLoggerHandler, PROMETHEUS_HANDLER
+
+
+class TestPrometheusCheckpointLoggerHandler(unittest.TestCase):
+
+    def setUp(self):
+        self.handler = PrometheusCheckpointLoggerHandler()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_TOTAL_BEGUN')
+    def test_log_begun(self, mock_begun):
+        self.handler.log_begun("test_flow")
+        mock_begun.labels.assert_called_once_with(flow="test_flow")
+        mock_begun.labels.return_value.inc.assert_called_once()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_TOTAL_FAILED')
+    def test_log_failure(self, mock_failed):
+        self.handler.log_failure("test_flow")
+        mock_failed.labels.assert_called_once_with(flow="test_flow")
+        mock_failed.labels.return_value.inc.assert_called_once()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_TOTAL_SUCCEEDED')
+    def test_log_success(self, mock_succeeded):
+        self.handler.log_success("test_flow")
+        mock_succeeded.labels.assert_called_once_with(flow="test_flow")
+        mock_succeeded.labels.return_value.inc.assert_called_once()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_TOTAL_ENDED')
+    def test_log_total_ended(self, mock_ended):
+        self.handler.log_total_ended("test_flow")
+        mock_ended.labels.assert_called_once_with(flow="test_flow")
+        mock_ended.labels.return_value.inc.assert_called_once()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_EVENTS')
+    def test_log_checkpoints(self, mock_events):
+        self.handler.log_checkpoints("test_flow", "test_checkpoint")
+        mock_events.labels.assert_called_once_with(flow="test_flow", checkpoint="test_checkpoint")
+        mock_events.labels.return_value.inc.assert_called_once()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_ERRORS')
+    def test_log_errors(self, mock_errors):
+        self.handler.log_errors("test_flow")
+        mock_errors.labels.assert_called_once_with(flow="test_flow")
+        mock_errors.labels.return_value.inc.assert_called_once()
+
+    @patch('helpers.checkpoint_logger.prometheus.CHECKPOINTS_SUBFLOW_DURATION')
+    def test_log_subflow(self, mock_subflow_duration):
+        self.handler.log_subflow("test_flow", "test_subflow", 10)
+        mock_subflow_duration.labels.assert_called_once_with(flow="test_flow", subflow="test_subflow")
+        mock_subflow_duration.labels.return_value.observe.assert_called_once_with(10)
+
+
+class TestCheckpointLogger(unittest.TestCase):
+
+    @patch('helpers.checkpoint_logger.PROMETHEUS_HANDLER')
+    @patch('sentry_sdk.set_measurement')
+    def test_submit_subflow(self, mock_set_measurement, mock_prometheus_handler):
+        # We need to mock the CheckpointLogger class and its methods
+        mock_checkpoint_logger = MagicMock()
+        mock_checkpoint_logger.cls.__name__ = "TestFlow"
+
+        # Mock the start and end events
+        mock_start = MagicMock()
+        mock_end = MagicMock()
+
+        # Set up the mock timing
+        mock_checkpoint_logger.timing.return_value = 5000  # 5 seconds in milliseconds
+
+        # Call the submit_subflow method
+        from helpers.checkpoint_logger import CheckpointLogger
+        CheckpointLogger.submit_subflow(mock_checkpoint_logger, "test_metric", mock_start, mock_end)
+
+        # Assert that sentry_sdk.set_measurement was called correctly
+        mock_set_measurement.assert_called_once_with("test_metric", 5000, "milliseconds")
+
+        # Assert that PROMETHEUS_HANDLER.log_subflow was called correctly
+        mock_prometheus_handler.log_subflow.assert_called_once_with(
+            flow="TestFlow", subflow="test_metric", duration=5.0
+        )
+
+
+if __name__ == '__main__':
+    unittest.main()

requirements.in

@@ -1,5 +1,5 @@
 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem
-https://github.com/codecov/shared/archive/b9e83b2c2f072749bae36c18b80d384b8cc50d94.tar.gz#egg=shared
+https://github.com/codecov/shared/archive/3cac029e52c1bc587f80b1e619103ce5ac4dee66.tar.gz#egg=shared
 https://github.com/codecov/test-results-parser/archive/ef39a0888acd62d02a316a852a15d755c74e78c6.tar.gz#egg=test-results-parser
 https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring
 asgiref>=3.7.2

requirements.txt

@@ -359,7 +359,7 @@ sentry-sdk[celery]==2.13.0
     # via
     #   -r requirements.in
     #   shared
-shared @ https://github.com/codecov/shared/archive/b9e83b2c2f072749bae36c18b80d384b8cc50d94.tar.gz
+shared @ https://github.com/codecov/shared/archive/3cac029e52c1bc587f80b1e619103ce5ac4dee66.tar.gz
     # via -r requirements.in
 six==1.16.0
     # via

services/processing/merging.py

@@ -38,6 +38,8 @@
 
     for intermediate_report in intermediate_reports:
         report = intermediate_report.report
+        if report.is_empty():
+            continue
 
         old_sessionid = next(iter(report.sessions))
         new_sessionid = master_report.next_session_number()

services/repository.py

@@ -24,9 +24,8 @@
 from shared.validation.exceptions import InvalidYamlException
 from shared.yaml import UserYaml
 from shared.yaml.user_yaml import OwnerContext
-from sqlalchemy.dialects.postgresql import insert
 from sqlalchemy.exc import IntegrityError
-from sqlalchemy.orm import Query
+from sqlalchemy.orm import Query, Session, lazyload
 
 from database.enums import CommitErrorTypes
 from database.models import Commit, Owner, Pull, Repository
@@ -530,47 +529,46 @@ async def _pick_best_base_comparedto_pair(
 
 @sentry_sdk.trace
 async def fetch_and_update_pull_request_information(
-    repository_service, db_session, repoid, pullid, current_yaml
+    repository_service,
+    db_session: Session,
+    repoid: int | str,
+    pullid: int | str,
+    current_yaml,
 ) -> EnrichedPull:
+    pull = (
+        db_session.query(Pull)
+        .options(lazyload("repository"))
+        .filter_by(pullid=pullid, repoid=repoid)
+        .first()
+    )
     try:
         pull_information = await repository_service.get_pull_request(pullid=pullid)
     except TorngitClientError:
         log.warning(
             "Unable to find pull request information on provider to update it due to client error",
             extra=dict(repoid=repoid, pullid=pullid),
         )
-        pull = db_session.query(Pull).filter_by(pullid=pullid, repoid=repoid).first()
         return EnrichedPull(database_pull=pull, provider_pull=None)
     except TorngitError:
         log.warning(
             "Unable to find pull request information on provider to update it due to unknown provider error",
             extra=dict(repoid=repoid, pullid=pullid),
         )
-        pull = db_session.query(Pull).filter_by(pullid=pullid, repoid=repoid).first()
         return EnrichedPull(database_pull=pull, provider_pull=None)
-    db_session.flush()
-    command = (
-        insert(Pull.__table__)
-        .values(
-            pullid=pullid,
+    if not pull:
+        pull = Pull(
             repoid=repoid,
-            issueid=pull_information["id"],
+            pullid=pullid,
             state=pull_information["state"],
             title=pull_information["title"],
+            issueid=pull_information["id"],
         )
-        .on_conflict_do_update(
-            index_elements=[Pull.repoid, Pull.pullid],
-            set_=dict(
-                issueid=pull_information["id"],
-                state=pull_information["state"],
-                title=pull_information["title"],
-            ),
-        )
-    )
-    db_session.connection().execute(command)
-    db_session.flush()
-    pull = db_session.query(Pull).filter_by(pullid=pullid, repoid=repoid).first()
-    db_session.refresh(pull)
+        db_session.add(pull)
+        db_session.flush()
+    else:
+        pull.state = pull_information["state"]
+        pull.title = pull_information["title"]
+        pull.issueid = pull_information["id"]
     base_commit_sha, compared_to = await _pick_best_base_comparedto_pair(
         repository_service, pull, current_yaml, pull_information
     )

tasks/tests/unit/test_upload_finisher_task.py

@@ -3,6 +3,8 @@
 from unittest.mock import ANY, call
 
 import pytest
+from celery.exceptions import Retry
+from redis.exceptions import LockError
 from shared.celery_config import timeseries_save_commit_measurements_task_name
 from shared.yaml import UserYaml
 
@@ -598,7 +600,7 @@ def test_finish_reports_processing_no_notification(
 
     @pytest.mark.django_db(databases={"default"})
     def test_upload_finisher_task_calls_save_commit_measurements_task(
-        self, mocker, dbsession, mock_redis
+        self, mocker, dbsession, mock_redis, mock_storage
     ):
         mocked_app = mocker.patch.object(
             UploadFinisherTask,
@@ -631,6 +633,32 @@ def test_upload_finisher_task_calls_save_commit_measurements_task(
             }
         )
 
+    @pytest.mark.django_db()
+    def test_retry_on_report_lock(self, dbsession, mock_redis):
+        commit = CommitFactory.create()
+        dbsession.add(commit)
+        dbsession.flush()
+
+        mock_redis.lock.side_effect = LockError()
+
+        task = UploadFinisherTask()
+        task.request.retries = 0
+
+        with pytest.raises(Retry):
+            task.run_impl(
+                dbsession,
+                [
+                    {
+                        "processings_so_far": [{"successful": True, "arguments": {}}],
+                        "parallel_incremental_result": {"upload_pk": 1},
+                    }
+                ],
+                repoid=commit.repoid,
+                commitid=commit.commitid,
+                commit_yaml={},
+                run_fully_parallel=True,
+            )
+
 
 class TestShouldCleanLabelsIndex(object):
     @pytest.mark.parametrize(