[← Back to Reviews Index](../Stewards%20Reviews%20Index.md)

# Python Test Review — Azure-AI-RAG-CSharp-Semantic-Kernel-Functions

**Steward:** Python Test Steward
**Project:** Azure-AI-RAG-CSharp-Semantic-Kernel-Functions
**Run date:** 2026-03-22

---

## 1. Test Suite Overview

| Attribute | Status |
|---|---|
| Test framework | None configured |
| pytest configuration | None (no `pytest.ini`, `pyproject.toml`, or `setup.cfg`) |
| `pytest-asyncio` | Not in requirements |
| Test files found | **0** |
| Total test count | **0** |

The project has **no Python test infrastructure whatsoever**. There are no test files, no pytest configuration, no test dependencies in `requirements.txt`, and no test runner configuration. A `test` directory is listed as ignored in `.funcignore`, but the directory itself does not exist.

---

## 2. Coverage Map

| Production Module | Path | Test File | Coverage Status | Priority |
|---|---|---|---|---|
| `function_app.py` — `Loader` (blob trigger handler) | `src/DocumentLoaderFunction/function_app.py` | None | No coverage | Critical |
| `function_app.py` — `AISearchIndexLoader.populate_search_index` | `src/DocumentLoaderFunction/function_app.py` | None | No coverage | Critical |
| `function_app.py` — `html_to_json` | `src/DocumentLoaderFunction/function_app.py` | None | No coverage | Notable |
| `directory_zipper.py` — `DirectoryZipper.copy_files` | `infra/scripts/directory_zipper.py` | None | No coverage | Minor |
| `directory_zipper.py` — `DirectoryZipper.create_zip` | `infra/scripts/directory_zipper.py` | None | No coverage | Minor |

Every Python module in the repository has zero test coverage.

---

## 3. Gap Analysis

### Priority 1 — Critical: Blob trigger handler (`Loader`)

`Loader` is the Azure Function entry point. It performs all of the following in a single function body:

- Reads blob content from the trigger input stream.
- Acquires an `DefaultAzureCredential` and fetches an AAD token, which it writes directly into environment variables (`OPENAI_API_TYPE`, `OPENAI_API_KEY`, `AZURE_OPENAI_AD_TOKEN`).
- Reads environment variables for search endpoint, index name, and OpenAI configuration.
- Calls `html_to_json`.
- Constructs an `AzureOpenAIEmbeddings` object.
- Instantiates and calls `AISearchIndexLoader.populate_search_index`.
- Moves the blob to a `completed` container.
- Deletes the source blob.

None of this logic is tested. The function contains a wide `except Exception` catch-all, so processing errors are silently swallowed. Without tests, there is no way to verify error-path behavior. This represents the highest-priority gap in the codebase.

**Specific untested scenarios:**
- Happy path: valid HTML blob processed end-to-end (all mocked).
- Blob with malformed HTML (no `<title>`, missing sections).
- `html_to_json` raises an exception.
- `populate_search_index` raises an exception — does the blob still get deleted?
- Blob deletion outside the `try` block at line 99–100: if an exception occurs during processing, `blob_service_client` is referenced outside the `try` block and will raise an `UnboundLocalError`.

### Priority 2 — Critical: `AISearchIndexLoader.populate_search_index`

The index-creation and document-upload logic is entirely untested. Untested paths:

- Index already exists (skips creation).
- Index does not exist (creates it).
- `embed_query` returns a valid embedding vector.
- `upload_documents` raises an exception (propagates via `raise ex`).
- `json_data` is missing expected keys (`reference_code`, `title`, `product_id`).

### Priority 3 — Notable: `html_to_json`

This is the only function with a clear, pure input/output contract. It takes raw bytes (HTML) and returns a `(dict, str)` tuple. It is the most straightforward function to unit test with no mocking required. Untested paths:

- Valid well-formed HTML document.
- HTML with no `<title>` tag.
- HTML with no `<h1>` tag.
- Missing sections (no "Description:", no "Reference Code:", etc.).
- Empty input or non-HTML input (will raise `AttributeError` on `.text` of `None`).
- HTML with multiple `<p>` tags inside sections.

---

## 4. Test Quality Assessment

### 4.1 Framework and Configuration

No `pytest.ini`, `pyproject.toml [tool.pytest.ini_options]`, or `setup.cfg [tool:pytest]` exists. There is no `conftest.py`. `pytest`, `pytest-asyncio`, `pytest-mock`, and `coverage` are absent from `requirements.txt`.

### 4.2 Naming

No tests exist to evaluate. The recommended naming pattern for this project is `test_should_{result}_when_{condition}`, e.g.:
- `test_should_process_blob_when_valid_html_uploaded`
- `test_should_skip_index_creation_when_index_already_exists`
- `test_should_return_no_title_when_html_has_no_title_tag`

### 4.3 Assertions

Not applicable — no tests exist.

### 4.4 Async Patterns

The `Loader` function and `AISearchIndexLoader` are synchronous. The Azure Functions Python v2 runtime handles invocation. No `async def` functions exist in production code. `pytest-asyncio` is not needed for the current codebase, but must be added if the function is refactored to async.

### 4.5 Mocking Strategy

No mocks exist. Any test added must mock:
- `azure.identity.DefaultAzureCredential` — to prevent AAD token acquisition.
- `azure.storage.blob.BlobServiceClient` — to prevent real blob storage calls.
- `azure.search.documents.SearchClient` — to prevent real AI Search calls.
- `azure.search.documents.indexes.SearchIndexClient` — to prevent real index management calls.
- `langchain_openai.AzureOpenAIEmbeddings.embed_query` — to prevent real OpenAI API calls.
- `os.environ` — to inject required environment variables in tests.
- `azure.functions.InputStream` — to simulate the blob trigger input.

### 4.6 Test Isolation

No shared mutable state issues can be evaluated — no tests exist. However, the production code mutates global `os.environ` directly (lines 49–54), which will cause test pollution if not properly isolated via `unittest.mock.patch.dict(os.environ, {...})` per test.

### 4.7 Structural Testability Issues

The `Loader` function mixes infrastructure setup (credential acquisition, client construction, environment mutation) with business logic (HTML parsing, embedding, indexing). This makes unit testing harder: to test any path, all infrastructure must be mocked. Extracting the business logic into a dedicated processor class or factory function would significantly improve testability, but that is a production code concern for the Python Best Practices Steward — noted here only insofar as it affects test strategy.

---

## 5. Findings

### PYTS-COVER-001 — Zero test coverage across all Python modules
**Severity:** Critical

No test files exist in the repository. Every production Python module — the blob trigger handler, the search index loader, the HTML parser, and the infrastructure script — has zero test coverage. There is no way to verify correctness of blob processing, HTML parsing, or search index population without manual deployment to Azure.

**File:** `src/DocumentLoaderFunction/function_app.py`
**Recommendation:** Create a `tests/` directory alongside `src/DocumentLoaderFunction/`. Add `pytest`, `pytest-mock`, and `coverage` to a `requirements-dev.txt`. Implement a minimum test suite covering: (1) `html_to_json` with valid and invalid HTML, (2) `AISearchIndexLoader.populate_search_index` with mocked search clients, (3) `Loader` handler with a mocked `InputStream` and all Azure clients mocked.

---

### PYTS-CONFIG-001 — No pytest configuration
**Severity:** Critical

There is no pytest configuration file (`pytest.ini`, `pyproject.toml`, or `setup.cfg`). Without this, CI runners cannot discover or execute tests reliably. Test paths, asyncio mode, and coverage reporting are unspecified.

**File:** `src/DocumentLoaderFunction/` (missing `pytest.ini` or `pyproject.toml`)
**Recommendation:** Add a `pyproject.toml` (or `pytest.ini`) at the `DocumentLoaderFunction` root with at minimum: `testpaths = ["tests"]`, `addopts = "--tb=short"`, and `asyncio_mode = "auto"` if async tests are introduced.

---

### PYTS-COVER-002 — Blob trigger handler `Loader` has no test coverage
**Severity:** Critical

The `Loader` function is the production entry point for all document ingestion. It contains multiple failure modes, including a reference to `blob_service_client` at lines 99–100 that is outside the `try` block, meaning an `UnboundLocalError` will occur if any exception is raised during the setup phase before that variable is assigned. This bug cannot be caught without a test that exercises the failure path.

**File:** `src/DocumentLoaderFunction/function_app.py`, line 99
**Recommendation:** Write a test `test_should_raise_unbound_error_when_exception_occurs_before_blob_client_assigned` to confirm and document the behavior. Then fix the scoping issue in the production code (flagged separately for the Python Best Practices Steward).

---

### PYTS-COVER-003 — `AISearchIndexLoader.populate_search_index` has no test coverage
**Severity:** Critical

The index creation and document upload logic is entirely untested. The function makes three external service calls (index check, optional index creation, document upload) and raises exceptions on embedding or upload failure. No test verifies any of these paths.

**File:** `src/DocumentLoaderFunction/function_app.py`, line 114
**Recommendation:** Add tests with mocked `SearchIndexClient` and `SearchClient` covering: index exists path, index does not exist path, successful document upload, and upload exception propagation.

---

### PYTS-MOCK-001 — No mocking infrastructure for Azure SDK or LangChain
**Severity:** Notable

The `requirements.txt` contains no test dependencies (`pytest-mock`, `responses`, `unittest.mock` is stdlib but needs explicit use). Any test that instantiates production code as-is would invoke real Azure services, breaking CI in environments without credentials.

**File:** `src/DocumentLoaderFunction/requirements.txt`
**Recommendation:** Create a `requirements-dev.txt` (or `requirements-test.txt`) containing `pytest`, `pytest-mock`, `pytest-cov`, and any Azure SDK test helpers. Document in the project README how to run tests locally.

---

### PYTS-COVER-004 — `html_to_json` has no tests despite being purely functional
**Severity:** Notable

`html_to_json` takes bytes input and returns a `(dict, str)` tuple with no external dependencies. It is the easiest function in the codebase to unit test. The lack of tests means that edge cases (missing HTML tags, empty input, malformed HTML) are unverified, and refactoring this function carries no safety net.

**File:** `src/DocumentLoaderFunction/function_app.py`, line 188
**Recommendation:** Add a `TestHtmlToJson` class with at minimum: valid HTML happy path, HTML missing `<title>`, HTML missing all optional sections, and empty bytes input.

---

### PYTS-ISOLATE-001 — Global `os.environ` mutation in `Loader` will cause test pollution
**Severity:** Notable

Lines 49–54 write directly to `os.environ` at runtime (`OPENAI_API_TYPE`, `OPENAI_API_KEY`, `AZURE_OPENAI_AD_TOKEN`). If tests invoke `Loader` without isolating the environment, these mutations will leak into other tests and affect global process state.

**File:** `src/DocumentLoaderFunction/function_app.py`, lines 49–54
**Recommendation:** Wrap all test invocations of `Loader` with `unittest.mock.patch.dict(os.environ, {}, clear=True)` or use a pytest fixture that saves/restores environment variables. Consider using the `pytest-env` or `monkeypatch` fixture for cleaner isolation.

---

### PYTS-COVER-005 — `DirectoryZipper` utility has no tests
**Severity:** Minor

`infra/scripts/directory_zipper.py` defines a `DirectoryZipper` class used in the deployment pipeline. It performs file system operations with no error handling. No tests verify exclusion logic, ZIP creation, or temp directory cleanup.

**File:** `infra/scripts/directory_zipper.py`
**Recommendation:** Add tests using `tmp_path` pytest fixture: verify files are copied, excluded dirs are skipped, excluded file extensions are skipped, and the temp directory is cleaned up after `create_zip`.

---

### PYTS-INFO-001 — `html_to_json` is well-structured for unit testing
**Severity:** Info

`html_to_json` has a clean signature, no side effects, and no external dependencies. It is an ideal starting point for introducing the test suite — low mocking overhead and high value as a regression baseline.

---

### PYTS-INFO-002 — `.funcignore` references a `test` directory, indicating test intent
**Severity:** Info

The `.funcignore` file lists `test` as an exclusion from Azure Functions deployment packages. This confirms the team intended to add tests. The directory itself does not exist yet.

---

## 6. Recommended Test Additions (Prioritized)

### Step 1 — Infrastructure setup (unlocks all other work)

1. Create `src/DocumentLoaderFunction/tests/__init__.py` (empty).
2. Create `src/DocumentLoaderFunction/tests/conftest.py` with shared fixtures:
   - `mock_blob_input`: a `MagicMock` mimicking `azure.functions.InputStream` with `.name`, `.length`, and `.read()`.
   - `mock_env`: a `monkeypatch`-based fixture injecting all required environment variables.
   - `mock_azure_clients`: patches for `DefaultAzureCredential`, `BlobServiceClient`, `SearchClient`, `SearchIndexClient`.
   - `mock_embeddings`: patches `AzureOpenAIEmbeddings.embed_query` to return a list of 1536 floats.
3. Create `pyproject.toml` (or `pytest.ini`) with `testpaths = ["tests"]`.
4. Add to `requirements-dev.txt`: `pytest>=8.0`, `pytest-mock>=3.12`, `pytest-cov>=5.0`.

### Step 2 — `html_to_json` unit tests (no mocks needed)

File: `tests/test_html_to_json.py`

| Test name | Scenario |
|---|---|
| `test_should_extract_all_fields_when_well_formed_html` | Full HTML with all sections |
| `test_should_return_no_title_when_title_tag_absent` | HTML without `<title>` |
| `test_should_return_empty_description_when_section_absent` | No "Description:" h2 |
| `test_should_return_no_reference_code_when_section_absent` | No "Reference Code:" h2 |
| `test_should_include_all_sections_in_text_data` | Verify text_data string concatenation |
| `test_should_raise_when_input_is_empty_bytes` | Edge: `b""` input |

### Step 3 — `AISearchIndexLoader` unit tests

File: `tests/test_ai_search_index_loader.py`

| Test name | Scenario |
|---|---|
| `test_should_create_index_when_index_does_not_exist` | `get_index` raises `ResourceNotFoundError` |
| `test_should_skip_index_creation_when_index_exists` | `get_index` returns successfully |
| `test_should_upload_document_with_embedding_when_valid_data` | Happy path, verify `upload_documents` called |
| `test_should_propagate_exception_when_upload_fails` | `upload_documents` raises, verify re-raise |
| `test_should_propagate_exception_when_embed_query_fails` | `embed_query` raises |
| `test_should_use_correct_field_names_in_uploaded_document` | Assert document dict keys |

### Step 4 — `Loader` blob trigger handler tests

File: `tests/test_loader.py`

| Test name | Scenario |
|---|---|
| `test_should_process_blob_when_valid_html_uploaded` | Full happy path with all mocks |
| `test_should_log_error_when_populate_search_index_raises` | Exception in processing caught by outer try |
| `test_should_move_blob_to_completed_when_processing_succeeds` | Verify `upload_blob` called on `completed` container |
| `test_should_delete_source_blob_after_processing` | Verify `delete_blob` called |
| `test_should_raise_unbound_local_error_when_credential_fails` | `DefaultAzureCredential` raises before `blob_service_client` assigned — documents existing bug |

### Step 5 — `DirectoryZipper` tests

File: `tests/test_directory_zipper.py`

| Test name | Scenario |
|---|---|
| `test_should_create_zip_with_all_files_when_no_exclusions` | Basic happy path using `tmp_path` |
| `test_should_exclude_directories_from_zip` | Verify excluded dirs are absent |
| `test_should_exclude_files_by_extension_from_zip` | Verify excluded extensions absent |
| `test_should_clean_up_temp_directory_after_zip_created` | Verify `temp_dir` removed |

---

## Summary

| Severity | Count |
|---|---|
| Critical | 4 |
| Notable | 3 |
| Minor | 1 |
| Info | 2 |
| **Total** | **10** |

The Python test posture for this project is at the lowest possible baseline: zero tests, zero configuration, zero test dependencies. The highest-priority action is establishing the test infrastructure (pytest config, dev requirements, conftest) and immediately following with `html_to_json` tests — the one function that requires no mocking and provides an immediate regression baseline. The blob trigger handler and search index loader tests are critical for production confidence but require a well-designed mock fixture set to remain isolated from real Azure services.