fixed unit tests

cg_lims/EPPs/udf/set/set_samples_reads_missing.py

@@ -1,12 +1,12 @@
 """CLI module to set Reads Missing (M) udf on new samples"""
 import logging
 import sys
-from typing import List
+from typing import List, Tuple
 
 import click
 from genologics.entities import Sample
 
-from cg_lims.exceptions import LimsError, MissingCgFieldError, MissingUDFsError
+from cg_lims.exceptions import LimsError, MissingUDFsError
 from cg_lims.get.samples import get_process_samples
 from cg_lims.status_db_api import StatusDBAPI
 
@@ -15,15 +15,15 @@
 FAILED = "FAILED"
 
 
-def set_reads_missing_on_sample(sample: Sample, status_db: StatusDBAPI):
+def set_reads_missing_on_sample(sample: Sample, status_db: StatusDBAPI) -> None:
     """ """
     app_tag = get_app_tag(sample)
     target_amount = get_target_amount(app_tag, status_db)
     sample.udf["Reads missing (M)"] = target_amount / 1000000
     sample.put()
 
 
-def get_app_tag(sample: Sample):
+def get_app_tag(sample: Sample) -> str:
     """ """
     try:
         return sample.udf["Sequencing Analysis"]
@@ -33,23 +33,28 @@ def get_app_tag(sample: Sample):
         )
 
 
-def get_target_amount(app_tag: str, status_db: StatusDBAPI):
+def get_target_amount(app_tag: str, status_db: StatusDBAPI) -> int:
     """ """
     try:
         return status_db.apptag(tag_name=app_tag, key="target_reads")
     except ConnectionError:
         raise LimsError(message="No connection to clinical-api!")
 
 
-def set_reads_missing(samples: List[Sample], status_db: StatusDBAPI):
+def set_reads_missing(
+    samples: List[Sample], status_db: StatusDBAPI
+) -> Tuple[int, List[str]]:
     """ """
-    failed_samples = 0
+    failed_samples_count = 0
+    failed_samples = []
     for sample in samples:
         try:
             set_reads_missing_on_sample(sample, status_db)
         except Exception:
-            failed_samples += 1
-    return failed_samples
+            failed_samples_count += 1
+            failed_samples.append(sample.id)
+
+    return failed_samples_count, failed_samples
 
 
 @click.command()
@@ -64,9 +69,13 @@ def set_reads_missing_on_new_samples(context: click.Context):
 
     try:
         samples = get_process_samples(process=process)
-        failed_samples = set_reads_missing(samples, status_db)
-        message = "Reads Missing (M) udf set on samples."
-        if failed_samples:
+        failed_samples_count, failed_samples = set_reads_missing(samples, status_db)
+        message = f"Reads Missing (M) udf set on samples."
+        if failed_samples_count:
+            message += (
+                f" Failed {failed_samples_count} samples: {', '.join(failed_samples)}"
+            )
+        if failed_samples_count:
             LOG.error(message)
             click.echo(message)
         else:

tests/EPPs/test_set_samples_reads_missing.py

@@ -16,25 +16,26 @@
 @mock.patch("cg_lims.EPPs.udf.set.set_samples_reads_missing.get_target_amount")
 @mock.patch("cg_lims.EPPs.udf.set.set_samples_reads_missing.get_app_tag")
 def test_set_reads_missing_on_sample(
-    mock_get_app_tag, mock_get_target_amount, mock_status_db, sample_1: Sample
+    mock_get_app_tag,
+    mock_get_target_amount,
+    mock_status_db,
+    sample_1: Sample,
 ):
-    """ """
     server("flat_tests")
 
     # GIVEN A SAMPLE
     sample = sample_1
 
-    # WHEN setting the missing reads on that sample_1
+    # WHEN setting the missing reads on that sample_1 based on the target amount
     mock_get_target_amount.return_value = 9_000_000
 
-    set_reads_missing_on_sample(sample_1, mock_status_db)
+    set_reads_missing_on_sample(sample, mock_status_db)
 
-    # THEN the udf "Reads missing (M)" on that sample should be set
-    assert sample_1.udf["Reads missing (M)"] == 9
+    # THEN the udf "Reads missing (M)" on that sample should be set correctly
+    assert sample.udf["Reads missing (M)"] == 9
 
 
 def test_get_app_tag(sample_1: Sample):
-    """ """
     # GIVEN a sample with a udf "Sequencing Analysis"
     sample_1.udf["Sequencing Analysis"] = "TESTAPPTAG"
 
@@ -46,14 +47,14 @@ def test_get_app_tag(sample_1: Sample):
 
 
 def test_get_app_tag_missing_udf(sample_1: Sample):
-    """ """
     # GIVEN a sample with a missing udf "Sequencing Analysis"
+    assert sample_1.udf.get("Sequencing Analysis") is None
 
     # WHEN getting the apptag for that sample
     with pytest.raises(MissingUDFsError) as error_message:
         get_app_tag(sample_1)
 
-    # THEN
+    # THEN the correct exception should be raised
     assert (
         f"UDF Sequencing Analysis not found on sample {sample_1.id}"
         in error_message.value.message
@@ -62,30 +63,28 @@ def test_get_app_tag_missing_udf(sample_1: Sample):
 
 @mock.patch("cg_lims.status_db_api.StatusDBAPI")
 def test_get_target_amount(mock_status_db):
-    """ """
     # GIVEN an apptag
     apptag = "TESTAPPTAG"
 
-    # WHEN fetching the target amount
+    # WHEN fetching the target amount from clinical-api
     mock_status_db.apptag.return_value = 9_000_000
     result = get_target_amount(apptag, mock_status_db)
 
-    # THEN the correct value should be returned
+    # THEN the get_target_amount should return that value
     assert result == 9_000_000
 
 
 @mock.patch("cg_lims.status_db_api.StatusDBAPI")
 def test_get_target_amount_connection_error(mock_status_db):
-    """ """
-    # GIVEN a connection error
+    # GIVEN a clinical-api can't be reached due to a connection error
     apptag = "TESTAPPTAG"
     mock_status_db.apptag.side_effect = ConnectionError
 
     # WHEN fetching the target amount
     with pytest.raises(LimsError) as error_message:
         get_target_amount(apptag, mock_status_db)
 
-    # THEN
+    # THEN the correct exception should be raised
     assert f"No connection to clinical-api!" in error_message.value.message
 
 
@@ -96,17 +95,19 @@ def test_get_target_amount_connection_error(mock_status_db):
 def test_set_reads_missing_one_sample(
     mock_status_db, mock_set_reads_missing_on_sample, sample_1: Sample
 ):
-    """ """
-
     # GIVEN a single sample
     samples = [sample_1]
 
-    # WHEN setting the reads missing on those samples
-    result = set_reads_missing(samples, status_db=mock_status_db)
+    # WHEN setting the reads missing on that sample
+    failed_samples_count, failed_samples = set_reads_missing(
+        samples, status_db=mock_status_db
+    )
 
-    # THEN
+    # THEN set_missing_reads should be called for that sample
     mock_set_reads_missing_on_sample.assert_called_with(sample_1, mock_status_db)
-    assert result == 0
+    # AND there should be no failed samples
+    assert failed_samples_count == 0
+    assert bool(failed_samples) is False
 
 
 @mock.patch(
@@ -116,46 +117,125 @@ def test_set_reads_missing_one_sample(
 def test_set_reads_missing_multiple_samples(
     mock_status_db, mock_set_reads_missing_on_sample, sample_1: Sample, sample_2: Sample
 ):
-    """ """
-
-    # GIVEN a list of samples
+    # GIVEN multiple of samples
     samples = [sample_1, sample_2]
 
     # WHEN setting the reads missing on those samples
-    result = set_reads_missing(samples, status_db=mock_status_db)
+    failed_samples_count, failed_samples = set_reads_missing(
+        samples, status_db=mock_status_db
+    )
 
     # THEN the missing reads should be set on both samples
-    assert mock_set_reads_missing_on_sample.call_count == 2
-    mock_set_reads_missing_on_sample.assert_has_calls(
-        [
-            mock.call(sample_1, mock_status_db),
-            mock.call(sample_2, mock_status_db),
-        ]
-    )
     assert mock_set_reads_missing_on_sample.mock_calls == [
         mock.call(sample_1, mock_status_db),
         mock.call(sample_2, mock_status_db),
     ]
-    assert result == 0
+    # AND there should be no failed samples
+    assert failed_samples_count == 0
+    assert bool(failed_samples) is False
 
 
 @mock.patch(
     "cg_lims.EPPs.udf.set.set_samples_reads_missing.set_reads_missing_on_sample"
 )
 @mock.patch("cg_lims.status_db_api.StatusDBAPI")
 def test_set_reads_missing_one_sample_exception(
-    mock_status_db, mock_set_reads_missing_on_sample, sample_1: Sample
+    mock_status_db,
+    mock_set_reads_missing_on_sample,
+    sample_1: Sample,
 ):
-    """ """
-
     # GIVEN a single sample
     samples = [sample_1]
-    mock_set_reads_missing_on_sample.side_effect = Exception
 
-    # WHEN setting the reads missing on those samples
-    with pytest.raises(Exception):
-        result = set_reads_missing(samples, mock_status_db)
+    # WHEN setting the reads missing on that sample leads to an exception  being raised
+    mock_set_reads_missing_on_sample.side_effect = Exception
+    failed_samples_count, failed_samples = set_reads_missing(samples, mock_status_db)
 
-    # THEN
+    # AND one failed sample should be counted, and it's id should be returned
     mock_set_reads_missing_on_sample.assert_called_with(sample_1, mock_status_db)
-    assert result == 1
+    assert failed_samples_count == 1
+    assert failed_samples == ["S1"]
+
+
+@mock.patch(
+    "cg_lims.EPPs.udf.set.set_samples_reads_missing.set_reads_missing_on_sample"
+)
+@mock.patch("cg_lims.status_db_api.StatusDBAPI")
+def test_set_reads_missing_multiple_samples_exception_on_first_sample(
+    mock_status_db,
+    mock_set_reads_missing_on_sample,
+    sample_1: Sample,
+    sample_2: Sample,
+):
+    # GIVEN multiple samples
+    samples = [sample_1, sample_2]
+
+    # WHEN setting the reads missing on those samples and the second sample leads to an exception being raised
+    mock_set_reads_missing_on_sample.side_effect = (None, Exception)
+    failed_samples_count, failed_samples = set_reads_missing(samples, mock_status_db)
+
+    # THEN setting the missing reads should be attempted for both samples
+    assert mock_set_reads_missing_on_sample.mock_calls == [
+        mock.call(sample_1, mock_status_db),
+        mock.call(sample_2, mock_status_db),
+    ]
+    # AND one failed sample should be counted, and it's id should be returned
+    assert failed_samples_count == 1
+    assert failed_samples == ["S2"]
+
+
+@mock.patch(
+    "cg_lims.EPPs.udf.set.set_samples_reads_missing.set_reads_missing_on_sample"
+)
+@mock.patch("cg_lims.status_db_api.StatusDBAPI")
+def test_set_reads_missing_multiple_samples_exception_on_second_sample(
+    mock_status_db,
+    mock_set_reads_missing_on_sample,
+    sample_1: Sample,
+    sample_2: Sample,
+):
+    # GIVEN multiple samples
+    samples = [sample_1, sample_2]
+    assert sample_1.udf.get("Sequencing Analysis") is None
+
+    # WHEN setting the reads missing on those samples and the first sample leads to an exception being raised
+
+    mock_set_reads_missing_on_sample.side_effect = (Exception, None)
+    failed_samples_count, failed_samples = set_reads_missing(samples, mock_status_db)
+
+    # THEN setting the missing reads should be attempted for both samples
+    assert mock_set_reads_missing_on_sample.mock_calls == [
+        mock.call(sample_1, mock_status_db),
+        mock.call(sample_2, mock_status_db),
+    ]
+    # AND one failed sample should be counted
+    assert failed_samples_count == 1
+    assert failed_samples == ["S1"]
+
+
+@mock.patch(
+    "cg_lims.EPPs.udf.set.set_samples_reads_missing.set_reads_missing_on_sample"
+)
+@mock.patch("cg_lims.status_db_api.StatusDBAPI")
+def test_set_reads_missing_multiple_samples_exception_on_both_samples(
+    mock_status_db,
+    mock_set_reads_missing_on_sample,
+    sample_1: Sample,
+    sample_2: Sample,
+):
+    # GIVEN multiple samples
+    samples = [sample_1, sample_2]
+    assert sample_1.udf.get("Sequencing Analysis") is None
+
+    # WHEN setting the reads missing on those samples and both samples leads to an exception being raised
+    mock_set_reads_missing_on_sample.side_effect = (Exception, Exception)
+    failed_samples_count, failed_samples = set_reads_missing(samples, mock_status_db)
+
+    # THEN setting the missing reads should be attempted for both samples
+    assert mock_set_reads_missing_on_sample.mock_calls == [
+        mock.call(sample_1, mock_status_db),
+        mock.call(sample_2, mock_status_db),
+    ]
+    # AND two failed sample should be counted, and their id's should be returned
+    assert failed_samples_count == 2
+    assert failed_samples == ["S1", "S2"]