diff --git a/swh/web/common/origin_save.py b/swh/web/common/origin_save.py --- a/swh/web/common/origin_save.py +++ b/swh/web/common/origin_save.py @@ -159,6 +159,15 @@ def _get_visit_info_for_save_request( save_request: SaveOriginRequest, ) -> Tuple[Optional[datetime], Optional[str]]: + """Retrieve visit information out of a save request + + Args: + save_request: Input save origin request to retrieve information for. + + Returns: + Tuple of (visit date, optional visit status) for such save request origin + + """ visit_date = None visit_status = None time_now = datetime.now(tz=timezone.utc) @@ -185,12 +194,23 @@ def _check_visit_update_status( save_request: SaveOriginRequest, save_task_status: str ) -> Tuple[Optional[datetime], str]: + """Given a save request and a save task status, determine whether a save request was + successful or failed. + + Args: + save_request: Input save origin request to retrieve information for. + + Returns: + Tuple of (optional visit date, save task status) for such save request origin + + """ visit_date, visit_status = _get_visit_info_for_save_request(save_request) save_request.visit_date = visit_date - # visit has been performed, mark the saving task as succeed if visit_date and visit_status is not None: + # visit has been performed, mark the saving task as succeeded save_task_status = SAVE_TASK_SUCCEEDED elif visit_status in ("created", "ongoing"): + # visit is currently running save_task_status = SAVE_TASK_RUNNING elif visit_status in ("not_found", "failed"): save_task_status = SAVE_TASK_FAILED @@ -209,12 +229,25 @@ task: Optional[Dict[str, Any]] = None, task_run: Optional[Dict[str, Any]] = None, ) -> Dict[str, Any]: + """Update save request information out of task and task_run information. + + Args: + save_request: Save request + task: Associated scheduler task information about the save request + task_run: Most recent run occurrence of the associated task + + Returns: + Summary of the save request information updated. + + """ must_save = False visit_date = save_request.visit_date + # save task still in scheduler db if task: save_task_status = _save_task_status[task["status"]] if task_run: + save_task_status = _save_task_run_status[task_run["status"]] # Consider request from which a visit date has already been found # as succeeded to avoid retrieving it again diff --git a/swh/web/tests/common/test_origin_save.py b/swh/web/tests/common/test_origin_save.py --- a/swh/web/tests/common/test_origin_save.py +++ b/swh/web/tests/common/test_origin_save.py @@ -1,4 +1,4 @@ -# Copyright (C) 2019-2020 The Software Heritage developers +# Copyright (C) 2019-2021 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU Affero General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -6,6 +6,7 @@ from datetime import datetime, timedelta, timezone from functools import partial import re +from typing import Optional import pytest import requests @@ -229,8 +230,12 @@ mock_get_origin_visits.assert_called_once() -def _get_save_origin_requests(mocker, load_status, visit_status): - # create a save request +def _get_save_origin_requests( + mocker, load_status, visit_status, request_date: Optional[datetime] = None +): + """Wrapper around the get_origin_save_origin_request call. + + """ SaveOriginRequest.objects.create( request_date=datetime.now(tz=timezone.utc), visit_type=_visit_type, @@ -256,7 +261,7 @@ formatted_date="", metadata={}, origin=_origin_url, - snapshot=None, + snapshot="", # make mypy happy status=visit_status, type=_visit_type, url="", @@ -271,9 +276,13 @@ @pytest.mark.django_db -def test_get_save_origin_requests_no_visit_date_found(mocker): +@pytest.mark.parametrize("visit_status", ["created", "ongoing",]) +def test_get_save_origin_requests_no_visit_date_found(mocker, visit_status): + """Uneventful visits with failed visit status are marked as failed + + """ sors = _get_save_origin_requests( - mocker, load_status="scheduled", visit_status="created" + mocker, load_status="scheduled", visit_status=visit_status, ) # check no visit date has been found assert len(sors) == 1 @@ -282,23 +291,57 @@ @pytest.mark.django_db -def test_get_save_origin_requests_failed_when_not_found(mocker): +@pytest.mark.parametrize("visit_status", ["not_found", "failed",]) +def test_get_save_origin_requests_no_failed_status_override(mocker, visit_status): + """Uneventful visits with failed statuses (failed, not found) are marked as failed + + """ sors = _get_save_origin_requests( - mocker, load_status="uneventful", visit_status="not_found" + mocker, load_status="uneventful", visit_status=visit_status ) assert len(sors) == 1 + + assert sors[0]["save_task_status"] == SAVE_TASK_FAILED + visit_date = sors[0]["visit_date"] + if visit_status == "failed": + assert visit_date is None + else: + assert visit_date is not None + + sors = get_save_origin_requests(_visit_type, _origin_url) + assert len(sors) == 1 assert sors[0]["save_task_status"] == SAVE_TASK_FAILED - assert sors[0]["visit_date"] is not None @pytest.mark.django_db -def test_get_save_origin_requests_no_failed_status_override(mocker): +@pytest.mark.parametrize( + "load_status,visit_status", + [("eventful", "full"), ("eventful", "partial"), ("uneventful", "partial"),], +) +def test_get_visit_info_for_save_request_succeeded(mocker, load_status, visit_status): + """Nominal scenario, below 30 days, returns something""" sors = _get_save_origin_requests( - mocker, load_status="uneventful", visit_status="not_found" + mocker, load_status=load_status, visit_status=visit_status ) assert len(sors) == 1 - assert sors[0]["save_task_status"] == SAVE_TASK_FAILED + + assert sors[0]["save_task_status"] == SAVE_TASK_SUCCEEDED assert sors[0]["visit_date"] is not None sors = get_save_origin_requests(_visit_type, _origin_url) - assert sors[0]["save_task_status"] == SAVE_TASK_FAILED + assert sors[0]["save_task_status"] == SAVE_TASK_SUCCEEDED + + +@pytest.mark.django_db +@pytest.mark.parametrize("load_status", ["eventful", "uneventful",]) +def test_get_visit_info_incomplete_visit_still_successful(mocker, load_status): + """Incomplete visit information, yet the task is considered ok + + """ + sors = _get_save_origin_requests( + mocker, load_status=load_status, visit_status=None, + ) + assert len(sors) == 1 + + assert sors[0]["save_task_status"] == SAVE_TASK_SUCCEEDED + assert sors[0]["visit_date"] is None