microsoft / microsoft/microsoft-ui-reactor

ApiIndexGeneratorTests.Index_IsUpToDate verifies one of the two committed API index copies, and its failure message recruits you into that narrower check

Open Beginner friendly
#1,035 4 comments 0 reactions 0 assignees View on GitHub
testing
Dominant language
C#
Stars
646
Forks
54
Avg merge
1d 3h
Merged PRs (30d)
84

Description

## Summary

The public API index is committed **twice**, and `ApiIndexGeneratorTests.Index_IsUpToDate` verifies **one** of them. That is already known and compensated for by a sibling test — but the failing test's own remediation string names `Index_IsUpToDate`, so anyone who follows the error message and stops there has verified half of what they believe they verified.

## Evidence

`tests/Reactor.Tests/Tooling/ApiIndexGeneratorTests.cs` @ `main` (`3f85ca49`):

```csharp
:38 static string CommittedIndexPath() => Path.Combine(RepoRoot(), "skills", "reactor.api.txt");

:40 static string[] OutputPaths() => new[]
{
:45 Path.Combine(root, "skills", "reactor.api.txt"),
:46 Path.Combine(root, "plugins", "reactor", "skills", "reactor-dsl", "references", "reactor.api.txt"),
};
```

| | mode | path source | copies covered |
|---|---|---|---|
| `Index_IsUpToDate` `:290` | update | `OutputPaths()` | **both** |
| `Index_IsUpToDate` `:298` | verify | `CommittedIndexPath()` | **one** |
| `Every_Committed_Copy_Matches_The_Generator` `:317` | verify | `OutputPaths()` | **both** |

The repo already documents the asymmetry, in the sibling test's own comment:

```csharp
:311 // The index is committed TWICE — `skills/` for `mur --api` / the agentkit NuGet
:312 // layout, and `plugins/.../references/` for the reactor-dsl skill. Both are packed
:313 // independently, and Index_IsUpToDate only ever compared the first, so the mirror
:314 // could drift silently and still ship.
```

## The actual problem: the remediation string

```csharp
:303 " $env:UPDATE_API_INDEX=1; dotnet test tests/Reactor.Tests
--filter \"FullyQualifiedName~Tooling.ApiIndexGeneratorTests.Index_IsUpToDate\" ..."
```

As an **update** selector this is correct — update mode writes both paths via `OutputPaths()`.

As a **verification** selector it covers one path. A contributor who hits the stale-index failure, runs the printed command to regenerate, then re-runs *the same filter* to confirm, has confirmed only `skills/reactor.api.txt`. The mirror under `plugins/reactor/skills/reactor-dsl/references/` is not compared on that path.

Running the whole class (`--filter "FullyQualifiedName~Tooling.ApiIndexGeneratorTests"`, 16 tests) is safe and does catch mirror drift. **The hazard is confined to narrowing by the `.Index_IsUpToDate` suffix — which is precisely what the failure text invites.**

## Suggested fix (one line, no test-logic change)

Change the remediation string at `:303` to select the **class** rather than the method:

```
--filter "FullyQualifiedName~Tooling.ApiIndexGeneratorTests"
```

Update mode still writes both paths, and re-running that filter to verify now covers both by construction — so nobody has to know about the asymmetry in order to be safe.

Optionally also rename `Index_IsUpToDate` to something that does not promise more than it checks (e.g. `PrimaryCommittedIndex_IsUpToDate`), leaving `Every_Committed_Copy_Matches_The_Generator` as the pair-level gate.

## Why it is worth fixing rather than documenting

A test name is the only thing most readers use to decide what a selector covers, and a failure message is the highest-trust instruction in the entire loop — it arrives at exactly the moment someone is looking for what to run. Correcting the string removes the trap at its source, and costs nothing.

## Notes

- Both committed copies are currently in sync (SHA256 `4A9E68F6…` on both), so this is a latent hazard rather than a live drift.
- `CommittedIndexPath()` is read at three sites; the third (`:83`, `ExistingSections_Unchanged`) is legitimately scoped — its subject is generator stability across a section span, for which a single copy is a valid sample. Only `:298` over-promises relative to its name.

Contributor guide

Open the contributing guide

Research direction

Open tests/Reactor.Tests/Tooling/ApiIndexGeneratorTests.cs and inspect the remediation string around line 303 in Index_IsUpToDate. Change the printed filter to select the full Tooling.ApiIndexGeneratorTests class, then run that class filter to verify the guidance covers both committed API index copies.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
testing-qa
Issue type
Bug
Difficulty
1/5
Estimated time
Under an hour
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.