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

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

| Field | Value |
|---|---|
| **Project** | Azure-AI-RAG-CSharp-Semantic-Kernel-Functions |
| **Run Date** | 2026-03-21 |
| **Steward** | C# Unit Test Steward |
| **Critical** | 1 |
| **Notable** | 3 |
| **Minor** | 2 |
| **Info** | 1 |
| **Total Findings** | 7 |

---

## 1. Test Suite Overview

**Test framework:** None detected.

The solution (`ChatAPI.sln`) contains a single C# project: `src/ChatAPI/ChatAPI.csproj`. There is **no test project** of any kind in the solution — no xUnit, NUnit, or MSTest project exists. No test files, no test references, and no test runner configuration were found anywhere in the C# workspace.

The only test file in the entire repository is `src/web/src/App.test.js`, which is a JavaScript/React test and is explicitly out of scope for this steward.

| Attribute | Value |
|---|---|
| Test framework | None |
| Test project count | 0 |
| Test file count | 0 (C# scope) |
| Production C# classes | 10 |
| Production C# public methods | ~14 |
| Tests covering production code | 0 |
| Overall C# test coverage | 0% |

---

## 2. Coverage Map

The table below maps every production class against test coverage. All items are completely untested.

| Production Class | Layer | File | Test File | Coverage Status | Priority |
|---|---|---|---|---|---|
| `ChatController` | Controller | `Controllers/ChatController.cs` | None | 🔴 Zero coverage | P1 — Critical |
| `SessionController` | Controller | `Controllers/SessionController.cs` | None | 🔴 Zero coverage | P1 — Critical |
| `ChatService` | Service | `Services/ChatService.cs` | None | 🔴 Zero coverage | P1 — Critical |
| `AISearchData` | Data | `Data/AISearchData.cs` | None | 🔴 Zero coverage | P2 — High |
| `ChatHistoryData` | Data | `Data/ChatHistoryData.cs` | None | 🔴 Zero coverage | P2 — High |
| `ProductData` | Data | `Data/ProductData.cs` | None | 🔴 Zero coverage | P2 — High |
| `AISearchDataPlugin` | Plugin | `Plugins/AISearchDataPlugin.cs` | None | 🔴 Zero coverage | P2 — High |
| `ProductDataPlugin` | Plugin | `Plugins/ProductDataPlugin.cs` | None | 🔴 Zero coverage | P2 — High |
| `GenerateProductInfo` | Data / Seeding | `Data/sample_data/products/GenerateProductInfo.cs` | None | 🔴 Zero coverage | P3 — Medium |
| `Product` (model) | Model | `Data/Product.cs` | None | N/A — data class | P4 — Low |

---

## 3. Gap Analysis

### 3.1 Highest-Priority Untested Classes

**Controllers (P1 — test now)**

- `ChatController.Post(ChatRequest)` — The sole chat endpoint. Accepts user input and calls `ChatService.GetResponseAsync`. No tests verify: null/empty request body, missing `SessionId`, service exception propagation, or the serialized response shape.
- `SessionController.GetSession()` — Generates a GUID-based session ID and returns it in an anonymous object. No tests verify the `session_id` field is present, that the value is non-empty, or that the format is a 32-character hex string.

**Service (P1 — test now)**

- `ChatService.GetResponseAsync(string, string)` — Core orchestration logic. Contains conditional branching on `chatHistory.Count == 1` to trigger history initialization. No tests verify the branching condition, the Cosmos/AI calls, or the final JSON serialization (`{ resp }`).

**Data layer (P2 — test soon)**

- `AISearchData.RetrieveDocumentationAsync` — Calls live Azure AI Search. The `SearchClient` is a concrete `sealed` class from the Azure SDK, which makes it difficult (but not impossible) to mock via wrapper or fake. No test verifies result mapping, empty-result handling, or exception propagation.
- `ChatHistoryData.AddMessageToHistoryAndSaveAsync` — Routes `"user"` and `"assistant"` roles to separate private methods. Neither branch is tested. The `"assistant"` message is saved with a `PartitionKey(chatMessage.id)` (not `sessionId`), which is a potential data access bug that tests would have caught.
- `ChatHistoryData.InitializeChatHistoryFromCosmosDBAsync` — Reconstitutes in-memory `ChatHistory` from Cosmos. No test verifies that messages are loaded in the correct order or that an empty session produces no side effects.
- `ProductData.GetProductAsync` — Not-found exception path is caught but re-thrown. No test verifies the re-throw behavior.
- `ProductData.GetProductByNameAsync` — Returns `null` when not found (nullable return type violation — method declares `Dictionary<string, object>` non-nullable but returns `null`). This is a latent null-reference bug that a test would immediately expose.

**Plugins (P2 — test soon)**

- `AISearchDataPlugin.ResourceLookup` — Generates embedding then calls `_aisearchData.RetrieveDocumentationAsync`. No test verifies exception re-throw or the embedding→search pipeline.
- `ProductDataPlugin.GetAzureProductDetailsById` — Wraps `ProductData.GetProductAsync` and serializes the result to JSON. No test verifies empty dictionary serialization, null product, or exception propagation.

### 3.2 Key Untested Behaviors

| Behavior | Location | Risk |
|---|---|---|
| `ChatService` branches on `chatHistory.Count == 1` | `ChatService.cs:34` | Logic bug if condition is evaluated on wrong state |
| `ProductData.GetProductByNameAsync` returns `null` (non-nullable signature) | `ProductData.cs:60` | NullReferenceException in callers |
| `ChatHistoryData` uses `PartitionKey(chatMessage.id)` instead of `sessionId` | `ChatHistoryData.cs:41,55` | Data retrieval mismatch — tests would reveal this |
| `ChatController.Post` has no null-guard on `request` | `ChatController.cs:20` | Unhandled NullReferenceException on null body |
| `GenerateProductInfo.PopulateCosmosAsync` swallows all exceptions silently | `GenerateProductInfo.cs:84` | Silent startup data loss; no test coverage |

---

## 4. Test Quality Assessment

Because no tests exist, this section evaluates what quality would be required if tests were written, and what patterns to avoid given the codebase structure.

### 4.1 Naming

No tests exist to assess naming. All new tests must follow the `Should_ExpectedResult_When_Condition` convention per project rules.

### 4.2 Assertions

No tests exist to assess assertions. Future tests must use specific `Assert.Equal` / `Assert.NotNull` assertions rather than empty "didn't throw" patterns.

### 4.3 Mocking Strategy Concerns

The production code's dependency graph presents specific testability challenges:

- `ChatService` takes **6 constructor parameters** (`Kernel`, `ITextEmbeddingGenerationService`, `ProductData`, `ChatHistory`, `ChatHistoryData`, `AISearchData`). Mocking 5+ dependencies violates the over-mocking threshold, which is a design signal that `ChatService` has too many direct responsibilities.
- `ProductData`, `ChatHistoryData`, and `GenerateProductInfo` accept `CosmosClient` directly (a concrete class). `CosmosClient` is not easily substitutable without a wrapping abstraction or the Azure SDK's in-memory emulator. This lack of interface seam is the root cause preventing easy unit testing of the data layer.
- `AISearchData` accepts `SearchClient` directly (another concrete Azure SDK class). Same substitution problem.
- None of the data classes implement an interface, meaning unit tests for `ChatService` and the plugins cannot easily isolate the data layer without refactoring.

### 4.4 Async Patterns

- All async methods in production code return `Task` or `Task<T>` — good baseline.
- No `async void` was found in production code.
- Future test methods must return `Task` (not `async void`) and use `await` on assertions.

### 4.5 Integration vs. Unit Test Separation

There is no test project at all, but when tests are added, the following separation must be enforced:

- Any test requiring a real `CosmosClient`, `SearchClient`, or Azure OpenAI endpoint is an **integration test** and must live in a separate project (e.g., `ChatAPI.IntegrationTests`).
- Pure unit tests (mocked dependencies) must live in a separate project (e.g., `ChatAPI.Tests`).

---

## 5. Findings

| Severity | ID | Title | File |
|---|---|---|---|
| 🔴 Critical | TEST-COVER-001 | No C# test project exists — zero test coverage | Solution-wide |
| 🟡 Notable | TEST-COVER-002 | Controllers have zero test coverage | `Controllers/ChatController.cs`, `Controllers/SessionController.cs` |
| 🟡 Notable | TEST-COVER-003 | ChatService has zero test coverage | `Services/ChatService.cs` |
| 🟡 Notable | TEST-ISOL-001 | Data classes use concrete Azure SDK types — no interface seam for unit testing | `Data/ProductData.cs`, `Data/AISearchData.cs`, `Data/ChatHistoryData.cs` |
| 🟢 Minor | TEST-COVER-004 | Plugins have zero test coverage | `Plugins/AISearchDataPlugin.cs`, `Plugins/ProductDataPlugin.cs` |
| 🟢 Minor | TEST-QUAL-001 | ChatService constructor has 6 dependencies — over-mocking threshold exceeded for unit tests | `Services/ChatService.cs` |
| ℹ️ Info | TEST-COVER-005 | GenerateProductInfo (data seeding) is untested and silently swallows errors | `Data/sample_data/products/GenerateProductInfo.cs` |

### TEST-COVER-001 — 🔴 Critical: No C# test project exists

**Description:** The solution contains exactly one C# project (`ChatAPI.csproj`), which is the production API project. There is no test project, no test framework reference, and no test files of any kind. This means 100% of the C# production codebase is completely untested.

**Impact:** Any regression, refactoring, or behavior change goes undetected. Critical paths (chat endpoint, session management, history reconstruction, product lookups) have no safety net.

**Recommendation:** Create a `ChatAPI.Tests` xUnit project. Add it to the solution. Reference `ChatAPI` and install `xunit`, `xunit.runner.visualstudio`, `Moq` (or `NSubstitute`), and `Microsoft.AspNetCore.Mvc.Testing` for controller integration tests.

---

### TEST-COVER-002 — 🟡 Notable: Controllers have zero test coverage

**Description:** `ChatController` and `SessionController` are the public API surface and have no tests.

- `ChatController.Post` can throw a `NullReferenceException` if `request` is null (no null guard before accessing `request.Input`).
- `SessionController.GetSession` is simple but its response shape (`session_id` field, hexadecimal format) is never validated.

**Recommendation:** Use `Microsoft.AspNetCore.Mvc.Testing.WebApplicationFactory` for integration-style controller tests, or use `ControllerBase` directly in unit tests with mocked `ChatService`.

---

### TEST-COVER-003 — 🟡 Notable: ChatService has zero test coverage

**Description:** `ChatService.GetResponseAsync` contains branching logic (`chatHistory.Count == 1`) that initializes chat history from Cosmos. This conditional is untested. The method also builds an `OpenAIPromptExecutionSettings` object inline and calls the Semantic Kernel chat completion service directly — all behavior that should be verified under test.

**Recommendation:** Extract the `IChatCompletionService` retrieval from the constructor or method so it can be mocked. Write tests for: (a) history initialization triggered on first call, (b) history initialization skipped on subsequent calls, (c) response serialized as `{ resp }` JSON, (d) exception propagation from the chat completion service.

---

### TEST-ISOL-001 — 🟡 Notable: Data classes use concrete Azure SDK types — no interface seam for unit testing

**Description:** `ProductData`, `ChatHistoryData`, and `AISearchData` accept `CosmosClient` and `SearchClient` directly. These are concrete classes from Azure SDKs with no publicly available mock-friendly interfaces. As a result, unit tests for any class that depends on these data classes (`ChatService`, plugins) cannot isolate the data layer without either (a) introducing interface abstractions or (b) using the Azure SDK's test emulators.

**Recommendation:** Introduce narrow interfaces (`IProductData`, `IChatHistoryData`, `IAISearchData`) that expose only the methods called by consumers. Register the concrete implementations in `Program.cs`. This creates a mockable seam for unit tests without requiring a live Azure environment.

---

### TEST-COVER-004 — 🟢 Minor: Plugins have zero test coverage

**Description:** `AISearchDataPlugin.ResourceLookup` and `ProductDataPlugin.GetAzureProductDetailsById` are Semantic Kernel functions invoked by the AI model. Their exception-catch-and-rethrow behavior and serialization logic are untested.

**Recommendation:** Once interface abstractions exist for the data layer (see TEST-ISOL-001), add tests for: (a) successful retrieval and serialization, (b) exception propagation, (c) empty result set handling.

---

### TEST-QUAL-001 — 🟢 Minor: ChatService constructor has 6 dependencies — over-mocking threshold exceeded

**Description:** `ChatService` accepts 6 constructor parameters: `Kernel`, `ITextEmbeddingGenerationService`, `ProductData`, `ChatHistory`, `ChatHistoryData`, `AISearchData`. Unit tests for this class would need to mock 5+ dependencies, which exceeds the over-mocking threshold (>3) and signals that `ChatService` has too many direct responsibilities.

**Recommendation:** Consider splitting `ChatService` into a slimmer orchestration class that delegates to a focused history service and a focused completion service. This would reduce the mock count in tests and improve cohesion.

---

### TEST-COVER-005 — ℹ️ Info: GenerateProductInfo silently swallows all exceptions

**Description:** `GenerateProductInfo.PopulateCosmosAsync` wraps the entire seeding operation in a top-level `catch (Exception ex)` that only logs and returns. If seeding fails, the application starts with an empty database and no indication of the failure at the HTTP layer. A test would expose this behavior immediately.

**Recommendation:** Consider letting critical startup failures propagate (or throw a custom startup exception) so the health check or startup probe can surface the issue. Add a test that verifies the exception is logged and not silently dropped in a way that masks data absence.

---

## 6. Recommended Test Additions (Prioritized)

| Priority | Finding Ref | Recommended Action | Effort |
|---|---|---|---|
| P1 | TEST-COVER-001 | Create `ChatAPI.Tests` xUnit project; add to solution; install xunit, Moq, and WebApplicationFactory | Low (scaffolding) |
| P1 | TEST-ISOL-001 | Introduce `IProductData`, `IChatHistoryData`, `IAISearchData` interfaces for mockable seams | Medium |
| P1 | TEST-COVER-002 | Add `ChatControllerTests`: null request guard, valid request returns serialized response, service exception propagates | Medium |
| P1 | TEST-COVER-002 | Add `SessionControllerTests`: response has `session_id` field, value is non-empty 32-char hex string | Low |
| P2 | TEST-COVER-003 | Add `ChatServiceTests`: history-init branching, response serialization, exception propagation | Medium |
| P2 | TEST-COVER-004 | Add `AISearchDataPluginTests` and `ProductDataPluginTests` after interface seams exist | Low |
| P3 | TEST-QUAL-001 | Refactor `ChatService` to reduce constructor dependency count to ≤4 | High |
| P3 | TEST-COVER-005 | Add startup test for `GenerateProductInfo` exception behavior | Low |

### Suggested Initial Test Structure

```
src/
  ChatAPI/                         (existing)
  ChatAPI.Tests/
    ChatAPI.Tests.csproj
    Controllers/
      ChatControllerTests.cs
      SessionControllerTests.cs
    Services/
      ChatServiceTests.cs
    Data/
      ProductDataTests.cs
      ChatHistoryDataTests.cs
      AISearchDataTests.cs
    Plugins/
      AISearchDataPluginTests.cs
      ProductDataPluginTests.cs
```

### Suggested Test Framework References

```xml
<PackageReference Include="xunit" Version="2.9.*" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.*" />
<PackageReference Include="Moq" Version="4.20.*" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="8.*" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.*" />
```

---

## Footer

This review is based on **static analysis** of source files only. No build was executed, no runtime behavior was observed, and no code coverage tooling was run. Findings reflect analysis of the source code as committed.

All findings are `open` (no `accepted-findings.json` was found in the workspace root).

**Steward:** C# Unit Test Steward | **Run date:** 2026-03-21 | **PREFIX:** TEST