WS-812: regression tests for bug fixes in MR683

apps/cli/executables/pexable/conveyor/conveyor/retrieve.py

@@ -15,6 +15,11 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with Workspaces.  If not, see <https://www.gnu.org/licenses/>.
+
+""" Post QA: Retrieve results directories from QA2 area to the spool area """
+
+# pylint: disable=E0401, E0402, W1203
+
 import glob
 import logging
 import os
@@ -23,21 +28,25 @@ from pathlib import Path
 from typing import Dict, List
 
 import requests
+from requests import Response
 
 from .interfaces import ConveyorIF
 
-"""
-Post QA: Retrieve results directories from QA2 area to the spool area
-"""
-
 
 class RetrievalConveyor(ConveyorIF):
+    """Retrieves results directories from QA2 area to the spool area"""
+
     def __init__(self, settings: Dict[str, str], action: str):
         self.logger = logging.getLogger("conveyor")
         self.action = action
         self.settings = settings
 
     def retrieval(self):
+        """
+        Get results from QA2 area and move them to spool dir
+
+        :return:
+        """
         qa_path = self.determine_qa_directory()
         spool_path = self.determine_spool_directory()
 
@@ -57,16 +66,26 @@ class RetrievalConveyor(ConveyorIF):
         if self.action == "QA Fail Cleanup":
             self.send_qa_fail_msg(self.settings["sdm_ids"])
 
-    def send_qa_fail_msg(self, sdm_id: str):
+    def send_qa_fail_msg(self, sdm_id: str) -> Response:
         """
         Call REST endpoint that shoots off QA Fail message
 
         :return:
         """
-        url = f"{self.settings['workflow_url']}/workflows/requests/{self.settings['request_id']}/send-do-not-calibrate/{sdm_id}"
-        requests.post(url, json={"sdm_id": sdm_id})
+        url = (
+            f"{self.settings['workflow_url']}/workflows/requests/"
+            f"{self.settings['request_id']}/send-do-not-calibrate/{sdm_id}"
+        )
+        return requests.post(url, json={"sdm_id": sdm_id})
 
     def break_symlinks(self, spool_path: Path, dir_list: List[str]):
+        """
+        Lose the symlinks we needed temporarily
+
+        :param spool_path:
+        :param dir_list:
+        :return:
+        """
         self.logger.info(f"Breaking symlinks between qa2 and spool for directory {spool_path.stem}...")
 
         for directory in dir_list:
@@ -78,6 +97,14 @@ class RetrievalConveyor(ConveyorIF):
                 self.logger.info(f'Directory "{directory}" is not a symlink! Skipping.')
 
     def move_subdirectories_to_spool(self, qa_path: Path, spool_path: Path, dir_list: List[str]):
+        """
+        Moves QA2 subdirectories to the request's spool dir.
+
+        :param qa_path:
+        :param spool_path:
+        :param dir_list:
+        :return:
+        """
         self.logger.info(f"Moving directories from qa2 to spool for directory {qa_path.stem}...")
         for directory in dir_list:
             qa_dir = qa_path / directory
@@ -87,6 +114,11 @@ class RetrievalConveyor(ConveyorIF):
             self.logger.info("Done.")
 
     def determine_qa_directory(self) -> Path:
+        """
+        Find the QA2 directory for this request.
+
+        :return:
+        """
         if "Calibration" in self.action:
             qa_area = self.settings["qa_delivery_area"]
         else:
@@ -96,24 +128,36 @@ class RetrievalConveyor(ConveyorIF):
         return Path(qa_area + "/" + wf_dir)
 
     def determine_spool_directory(self) -> Path:
+        """
+        Find the spool directory for this request.
+
+        :return:
+        """
         spool_area = self.settings["workspaces_lustre_root_dir"]
         wf_dir = self.settings["destination_subdir"]
         return Path(spool_area + "/" + wf_dir)
 
     def check_spool_contents(self, spool_path: Path, expected_dirs: List[str]) -> bool:
+        """
+        Make sure request's spool dir has everything we'd expect for a successful retrieval.
+
+        :param spool_path:
+        :param expected_dirs:
+        :return:
+        """
         os.chdir(spool_path)
 
         contents = glob.glob("*/", recursive=True)
         if set(contents).issubset(expected_dirs) and not len(contents) < len(expected_dirs):
             self.logger.info("All directories were successfully transferred!")
             return True
-        else:
-            self.logger.error(
-                f"ERROR: A directory seems to be missing! "
-                f"Directory list {contents} does not match the "
-                f"list of expected directories!"
-            )
-            return False
+
+        self.logger.error(
+            f"ERROR: A directory seems to be missing! "
+            f"Directory list {contents} does not match the "
+            f"list of expected directories!"
+        )
+        return False
 
     def convey(self):
         self.logger.info(f"RUNNING POST QA STANDARD {self.action.upper()}!")

apps/cli/executables/pexable/conveyor/test/test_conveyor.py

@@ -18,12 +18,19 @@
 """
 Tests for conveyor.conveyor module
 """
+
+# pylint: disable=R0201, W0212
 import argparse
+import copy
 import logging
 import sys
+from http import HTTPStatus
 from unittest.mock import MagicMock, patch
 
 import conveyor.conveyor as con
+from conveyor.retrieve import RetrievalConveyor
+from pyramid.response import Response
+from urllib3.connection import HTTPConnection
 
 logger = logging.getLogger("test_conveyer")
 logger.setLevel(logging.INFO)
@@ -39,15 +46,25 @@ expected_settings = {
     "destination_subdir": "tmpabcd1234",
     "system_id": 2,
     "sdm_ids": "brain_000.58099.67095825232",
+    "workflow_url": "http://fake:1234",
 }
 
 args = argparse.Namespace(deliver=None, retrieve=None, deliver_img=None, retrieve_img=None, qa_fail=None)
 
 TEST_JSON = "test/test.json"
+PARSE_ARGS_PATCH = "argparse.ArgumentParser.parse_args"
+CONVEY_PATCH = "conveyor.retrieve.RetrievalConveyor.convey"
 
 
 class TestConveyor:
+    """Tests for conveyor"""
+
     def test_get_settings(self):
+        """
+        Be sure all the settings needed get set.
+
+        :return:
+        """
         with patch("pathlib.Path.cwd") as cwd:
             settings = con._get_settings(TEST_JSON)
             assert settings["qa_delivery_area"] == expected_settings["qa_delivery_area"]
@@ -59,11 +76,17 @@ class TestConveyor:
             assert settings["destination_subdir"] == expected_settings["destination_subdir"]
             assert settings["request_id"] == expected_settings["system_id"]
             assert settings["sdm_ids"] == expected_settings["sdm_ids"]
+            assert settings["workflow_url"] is not None
 
     def test_main_deliver(self):
+        """
+        Confirm that DeliveryConveyor actually conveys.
+
+        :return:
+        """
         args.deliver = [TEST_JSON]
 
-        with patch("argparse.ArgumentParser.parse_args", MagicMock(return_value=args)):
+        with patch(PARSE_ARGS_PATCH, MagicMock(return_value=args)):
             assert args.deliver[0] == TEST_JSON
             with patch("conveyor.deliver.DeliveryConveyor.convey") as del_convey:
                 con.main()
@@ -73,10 +96,15 @@ class TestConveyor:
         args.deliver = None
 
     def test_main_retrieve(self):
+        """
+        Confirm that DeliveryConveyor retrieves what it's meant to convey.
+
+        :return:
+        """
         args.retrieve = [TEST_JSON]
 
-        with patch("argparse.ArgumentParser.parse_args", MagicMock(return_value=args)):
-            with patch("conveyor.retrieve.RetrievalConveyor.convey") as ret_convey:
+        with patch(PARSE_ARGS_PATCH, MagicMock(return_value=args)):
+            with patch(CONVEY_PATCH) as ret_convey:
                 con.main()
                 assert ret_convey.call_count == 1
 
@@ -90,15 +118,38 @@ class TestConveyor:
         :return:
         """
         args.qa_fail = [TEST_JSON]
-        args.fileSetIds = "brain_000.58099.67095825232"
+        args.fileSetIds = expected_settings["sdm_ids"]
 
         try:
             with patch("os.chdir"):
-                with patch("argparse.ArgumentParser.parse_args", MagicMock(return_value=args)):
-                    with patch("conveyor.retrieve.RetrievalConveyor.convey") as mock_convey:
+                with patch(PARSE_ARGS_PATCH, MagicMock(return_value=args)):
+                    with patch(CONVEY_PATCH) as mock_convey:
                         con.main()
                         assert mock_convey.call_count == 1
         finally:
             # reset for other testing
             args.qa_fail = None
             args.fileSetIds = None
+
+    def test_send_msg_url_is_valid(self):
+        """
+         Check that send_qa_fail_message posts a valid URL.
+
+        :return:
+        """
+        settings = copy.copy(expected_settings)
+        settings["request_id"] = -1
+        r_con = RetrievalConveyor(settings, "qa_fail")
+
+        expected_url = (
+            f"{settings['workflow_url']}/workflows/requests/{settings['request_id']}/"
+            f"send-do-not-calibrate/{expected_settings['sdm_ids']}"
+        )
+        expected_response = Response(status_code=HTTPStatus.OK, json_body={"url": expected_url})
+        http_conn = HTTPConnection("localhost", -1)
+        with patch("requests.post", return_value=expected_response):
+            with patch("urllib3.connection.HTTPConnection.connect", return_value=http_conn):
+
+                response = r_con.send_qa_fail_msg(expected_settings["sdm_ids"])
+                assert response.status_code == expected_response.status_code
+                assert response.body == expected_response.body

services/workflow/test/test_workflow_service_rest_api.py

@@ -17,18 +17,17 @@
 # along with Workspaces.  If not, see <https://www.gnu.org/licenses/>.
 """ Tests for WorkflowRequestRestService """
 
-# pylint: disable=E0401, W0621
-
+# pylint: disable=C0103, E0401, W0621
 import datetime
 import http
 import logging
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
 from pyramid.testing import DummyRequest
-from workflow.server import WorkflowRequestRestService
+from workflow.server import WorkflowRequestRestService, lookup_file
 
-from workspaces.workflow.schema import Workflow, WorkflowRequest
+from workspaces.workflow.schema import Workflow, WorkflowRequest, WorkflowRequestFile
 from workspaces.workflow.services.workflow_info import WorkflowInfo
 from workspaces.workflow.services.workflow_service import WorkflowService
 
@@ -169,6 +168,12 @@ def test_announces_qa(workflow_request: DummyRequest):
 
 
 def test_send_carta_url_to_aat(workflow_request_request: DummyRequest):
+    """
+    Get CARTA URL resulting from this request to the AAT-PPI
+
+    :param workflow_request_request:
+    :return:
+    """
     # Init dummy request values
     matchdict_savepoint = workflow_request_request.matchdict
     body_savepoint = workflow_request_request.json_body
@@ -218,6 +223,12 @@ def test_send_do_not_calibate_msg_to_aat(workflow_request_request: DummyRequest)
 
 
 def test_get_request_htcondor_id(workflow_request_request: DummyRequest):
+    """
+    Get the HTCondor ID for this request.
+
+    :param workflow_request_request:
+    :return:
+    """
     matchdict_savepoint = workflow_request_request.matchdict
     workflow_request_request.matchdict["request_id"] = -1
 
@@ -227,3 +238,34 @@ def test_get_request_htcondor_id(workflow_request_request: DummyRequest):
 
     # Reset dummy request properties to their initial values
     workflow_request_request.matchdict = matchdict_savepoint
+
+
+def test_lookup_file_behaves_as_expected(workflow_request_request: DummyRequest):
+    """
+    If the request has a file attached to it, lookup_file() should find it;
+    if there are no files, it should return nothing
+
+    :param workflow_request_request:
+    :return:
+    """
+    matchdict_savepoint = workflow_request_request.matchdict
+
+    filename = "metadata.json"
+    workflow_request_request.matchdict["filename"] = filename
+    workflow_request_request.matchdict["request_id"] = -1
+    wrf = WorkflowRequestFile(filename=filename)
+    wf_req = WorkflowRequest(workflow_request_id=-1, workflow_name="qa_fail", files=[wrf])
+
+    with patch("workflow.server.lookup_request", return_value=wf_req):
+        result = lookup_file(workflow_request_request)
+        # it should find our workflow request file
+        assert result == wrf
+
+    wf_req = WorkflowRequest(workflow_request_id=-1, workflow_name="qa_fail", files=[])
+    with patch("workflow.server.lookup_request", return_value=wf_req):
+        # there's no file to look up
+        result = lookup_file(workflow_request_request)
+        assert result is None
+
+    # Reset dummy request
+    workflow_request_request.matchdict = matchdict_savepoint