add tests

Author: arslanashraf7

learning_resources/content_summarizer.py

@@ -128,15 +128,15 @@ def get_unprocessed_content_file_ids(
 
     def summarize_content_files_by_ids(
         self, content_file_ids: list[int], overwrite
-    ) -> None:
+    ) -> list[str]:
         """Process multiple content files by id.
 
         Args:
             - ids (list[int]): List of content file ids to process
             - overwrite (bool): Whether to overwrite existing summary and flashcards
 
         Returns:
-            - None
+            - list[str]: List of status messages for each content file
         """
         status_messages = []
         for content_file_id in content_file_ids:
@@ -150,7 +150,7 @@ def summarize_single_content_file(
         self,
         content_file_id: int,
         overwrite,
-    ) -> tuple[bool, str]:
+    ) -> str:
         """Process a single content file
         Args:
             - content_file_id (int): Id of the content file to process
@@ -184,19 +184,23 @@ def summarize_single_content_file(
 
                     if updated:
                         content_file.save()
-                    return f"Content file summarization succeeded for CONTENT_FILE_ID: {content_file_id}"  # noqa: E501
-                return f"Content file summarization skipped for CONTENT_FILE_ID: {content_file_id}"  # noqa: E501
-
+                    return f"Summarization succeeded for CONTENT_FILE_ID: {content_file_id}"  # noqa: E501
+                return f"Summarization skipped for CONTENT_FILE_ID: {content_file_id}"
         except SummaryGenerationError as exc:
-            return f"Content file summary generation failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n"  # noqa: E501
-
+            # Log and return a specific readable error message when summary
+            # generation fails.
+            logger.exception("Error processing content: %d", content_file.id)
+            return f"Summary generation failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n"  # noqa: E501
         except FlashcardsGenerationError as exc:
-            return f"Content file flashcards generation failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n"  # noqa: E501
+            # Log and return a specific readable error message when flashcards
+            # generation fails.
+            return f"Flashcards generation failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n"  # noqa: E501
         except Exception as exc:
+            # Log and return a specific readable error message when an  unknown
+            # error occurs.
             logger.exception("Error processing content: %d", content_file.id)
             return (
-                False,
-                f"Content file summarization failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n",  # noqa: E501
+                f"Summarization failed for CONTENT_FILE_ID: {content_file_id}\nError: {exc.args[0]}\n\n",  # noqa: E501
             )
 
     def _get_llm(self, model=None, temperature=0.0, max_tokens=1000) -> ChatLiteLLM:
@@ -236,13 +240,16 @@ def _generate_summary(self, content: str, llm_model: str) -> str:
             logger.info("Generated summary: %s", generated_summary)
 
         except Exception as exc:
+            # We do not want to raise the exception as is, we will log the exception and
+            # raise SummaryGenerationError that will be used to make further decisions
+            # in the code.
             logger.exception(
                 "An error occurred while generating summary using model: %s", llm_model
             )
             raise SummaryGenerationError(exc) from exc
 
         else:
-            return True, generated_summary
+            return generated_summary
 
     def _generate_flashcards(
         self, content: str, llm_model: str
@@ -264,11 +271,13 @@ def _generate_flashcards(
             generated_flashcards = response.get("flashcards")
             logger.info("Generated flashcards: %s", generated_flashcards)
         except Exception as exc:
+            # We do not want to raise the exception as is, we will log the exception and
+            # raise FlashcardsGenerationError that will be used to make further
+            # decisions in the code.
             logger.exception(
                 "An error occurred while generating flashcards using model: %s",
                 llm_model,
             )
             raise FlashcardsGenerationError(exc) from exc
-
         else:
             return generated_flashcards

learning_resources/content_summarizer_test.py

@@ -6,6 +6,10 @@
     PlatformType,
 )
 from learning_resources.content_summarizer import ContentSummarizer
+from learning_resources.exceptions import (
+    FlashcardsGenerationError,
+    SummaryGenerationError,
+)
 from learning_resources.factories import (
     ContentFileFactory,
     ContentSummarizerConfigurationFactory,
@@ -242,9 +246,9 @@ def test_get_unprocessed_content_files_with_platform_and_config(
 def test_summarize_content_files_by_ids(
     processable_content_files, mock_summarize_single_content_file
 ):
-    """The summarizer should process content files that are processable"""
+    """The summarizer should process content files that are processable and return the status results"""
     summarizer = ContentSummarizer()
-    summarizer.summarize_content_files_by_ids(
+    results = summarizer.summarize_content_files_by_ids(
         overwrite=False,
         content_file_ids=[
             content_file.id for content_file in processable_content_files
@@ -253,6 +257,8 @@ def test_summarize_content_files_by_ids(
     assert mock_summarize_single_content_file.call_count == len(
         processable_content_files
     )
+    assert isinstance(results, list)
+    assert len(results) == len(processable_content_files)
 
 
 def test_summarize_single_content_file(mocker, processable_content_files):
@@ -332,3 +338,77 @@ def test_process_single_file_calls_llm_summary(
         assert mock_instance.with_structured_output.call_count == 1
     elif has_flashcards:
         assert mock_instance.invoke.call_count == 1
+
+
+@pytest.mark.parametrize(
+    ("process_type", "expected_exception"),
+    [("summary", SummaryGenerationError), ("flashcards", FlashcardsGenerationError)],
+)
+def test_generate_summary_flashcards_exception(
+    mocker, processable_content_files, settings, process_type, expected_exception
+):
+    """Test the exception handling in the generate_summary and generate_flashcards methods"""
+    settings.OPENAI_API_KEY = "test"
+    summarizer = ContentSummarizer()
+    content_file = processable_content_files[0]
+    content_file.save()
+
+    # Mock the ChatLiteLLM class and its methods
+    mock_chat_llm = mocker.patch(
+        "learning_resources.content_summarizer.ChatLiteLLM", autospec=True
+    )
+    mock_instance = mock_chat_llm.return_value
+
+    # Mock the response for _generate_summary to raise an exception
+    mock_instance.invoke.side_effect = Exception("Test exception")
+    # Mock the response for _generate_flashcards to raise an exception
+    mock_instance.with_structured_output.return_value.invoke.side_effect = Exception(
+        "INVALID_FORMAT"
+    )
+
+    if process_type == "summary":
+        with pytest.raises(expected_exception):
+            summarizer._generate_summary(  # noqa: SLF001
+                llm_model="llm_model", content=content_file.content
+            )
+    else:
+        with pytest.raises(expected_exception):
+            summarizer._generate_flashcards(  # noqa: SLF001
+                llm_model="llm_model", content=content_file.content
+            )
+
+
+def test_summarize_single_content_file_with_exception(
+    mocker, processable_content_files, settings
+):
+    """Test the exception handling in the summarize_single_content_file method"""
+    settings.OPENAI_API_KEY = "test"
+    summarizer = ContentSummarizer()
+    content_file = processable_content_files[0]
+
+    # Mock the ChatLiteLLM class and its methods
+    mock_chat_llm = mocker.patch(
+        "learning_resources.content_summarizer.ChatLiteLLM", autospec=True
+    )
+    mock_instance = mock_chat_llm.return_value
+
+    # Mock the response for _generate_summary to raise an exception
+    mock_instance.invoke.side_effect = Exception("Test exception")
+    # Mock the response for _generate_flashcards to raise an exception
+    mock_instance.with_structured_output.return_value.invoke.side_effect = Exception(
+        "INVALID_FORMAT"
+    )
+
+    error = summarizer.summarize_single_content_file(content_file.id, overwrite=False)
+    assert (
+        error
+        == f"Summary generation failed for CONTENT_FILE_ID: {content_file.id}\nError: Test exception\n\n"
+    )
+    content_file.summary = "Test summary"
+    content_file.save()
+    content_file.refresh_from_db()
+    error = summarizer.summarize_single_content_file(content_file.id, overwrite=False)
+    assert (
+        error
+        == f"Flashcards generation failed for CONTENT_FILE_ID: {content_file.id}\nError: INVALID_FORMAT\n\n"
+    )