Address comments in PR #373

* Fix cassette to handle method call to refresh index
* Use parameterized tests
* Remove comments

Author: jonavellecuerdo

tests/fixtures/cassettes/opensearch/bulk_update_updates_records.yaml

@@ -26,4 +26,26 @@ interactions:
     status:
       code: 200
       message: OK
+- request:
+    body: null
+    headers:
+      Content-Length:
+      - '0'
+      content-type:
+      - application/json
+      user-agent:
+      - opensearch-py/2.8.0 (Python 3.12.2)
+    method: POST
+    uri: http://localhost:9200/test-index/_refresh
+  response:
+    body:
+      string: '{"_shards":{"total":2,"successful":1,"failed":0}}'
+    headers:
+      content-length:
+      - '49'
+      content-type:
+      - application/json; charset=UTF-8
+    status:
+      code: 200
+      message: OK
 version: 1

tests/test_cli.py

@@ -1,4 +1,5 @@
 import json
+import pytest
 import re
 from unittest.mock import patch
 
@@ -274,13 +275,25 @@ def test_bulk_update_with_source_raise_bulk_indexing_error(
     )
 
 
+@pytest.mark.parametrize(
+    "bulk_update_return",
+    [
+        {"updated": 1, "errors": 0, "total": 1},
+        {"updated": 0, "errors": 1, "total": 1},
+    ],
+)
 @patch("tim.helpers.validate_bulk_cli_options")
 @patch("tim.opensearch.bulk_update")
-def test_bulk_update_embeddings_success(
-    mock_bulk_update, mock_validate_bulk_cli_options, caplog, monkeypatch, runner
+def test_bulk_update_embeddings_logs_complete(
+    mock_bulk_update,
+    mock_validate_bulk_cli_options,
+    bulk_update_return,
+    caplog,
+    monkeypatch,
+    runner,
 ):
     monkeypatch.delenv("TIMDEX_OPENSEARCH_ENDPOINT", raising=False)
-    mock_bulk_update.return_value = {"updated": 1, "errors": 0, "total": 1}
+    mock_bulk_update.return_value = bulk_update_return
     mock_validate_bulk_cli_options.return_value = "libguides"
 
     result = runner.invoke(
@@ -304,34 +317,7 @@ def test_bulk_update_embeddings_success(
 
 @patch("tim.helpers.validate_bulk_cli_options")
 @patch("tim.opensearch.bulk_update")
-def test_bulk_update_embeddings_raise_error(
-    mock_bulk_update, mock_validate_bulk_cli_options, caplog, monkeypatch, runner
-):
-    monkeypatch.delenv("TIMDEX_OPENSEARCH_ENDPOINT", raising=False)
-    mock_bulk_update.return_value = {"updated": 0, "errors": 1, "total": 1}
-    mock_validate_bulk_cli_options.return_value = "libguides"
-
-    result = runner.invoke(
-        main,
-        [
-            "bulk-update-embeddings",
-            "--source",
-            "libguides",
-            "--run-id",
-            "85cfe316-089c-4639-a5af-c861a7321493",
-            "tests/fixtures/dataset",
-        ],
-    )
-    assert result.exit_code == EXIT_CODES["success"]
-    assert (
-        f"Bulk update with embeddings complete: {json.dumps(mock_bulk_update())}"
-        in caplog.text
-    )
-
-
-@patch("tim.helpers.validate_bulk_cli_options")
-@patch("tim.opensearch.bulk_update")
-def test_bulk_update_embeddings_raise_bulk_operation_error(
+def test_bulk_update_embeddings_exit_bulk_operation_error(
     mock_bulk_update, mock_validate_bulk_cli_options, caplog, monkeypatch, runner
 ):
     monkeypatch.delenv("TIMDEX_OPENSEARCH_ENDPOINT", raising=False)
@@ -351,7 +337,7 @@ def test_bulk_update_embeddings_raise_bulk_operation_error(
             "tests/fixtures/dataset",
         ],
     )
-    assert result.exit_code == EXIT_CODES["success"]
+    assert result.exit_code == EXIT_CODES["error"]
     assert "Bulk update with embeddings failed" in caplog.text
 
 

tests/test_opensearch.py

@@ -535,10 +535,6 @@ def test_bulk_delete_logs_error_if_record_not_found(
     )
 
 
-# what happens when you try to 'bulk_update_embeddings' a document that doesn't exist*
-#   - do you need all these mocks or VCRs to test that (probably no :o)
-# fixture that reindexes a source into the mock
-# if we run tda in fixtures/datasets -- what's in it??
 @my_vcr.use_cassette("opensearch/bulk_update_updates_records.yaml")
 def test_bulk_update_updates_records(test_opensearch_client):
     updates = [