Add incremental edit tests — verify editing one file only regenerates affected contracts#214
Conversation
…tateTransition Add RunIncrementalEdit helper to SourceGeneratorTestContext that runs a generator on an initial compilation, replaces one syntax tree with a modified version, and re-runs. This enables testing the core incremental value: editing one file should only regenerate affected contracts. Three new test methods in IncrementalCacheTests: - RegisterStrategy_EditOneFile_UnmodifiedContractStaysCached: two strategy contracts in separate files, add a strategy to one file, assert Combine is Modified and transform reflects the new strategy. - GenerateSingleton_EditOneFile_TransformReflectsChange: two singleton targets in separate files, add a singleton to one file, assert transform has at least one Modified/New output. - StateTransition_EditOneFile_CollectIsModified: two state machines in separate files, add a transition to one file, assert Combine is Modified and transform reflects the change. Also add GetTrackedStepReasons and GetAllTrackedStepNames helpers for inspecting tracked step reasons from a GeneratorDriverRunResult. Closes #213
Skymly
left a comment
There was a problem hiding this comment.
Review: PR #214 — Incremental edit tests
Overall this is a solid addition — the RunIncrementalEdit helper and 3 edit tests fill an important gap in the incremental cache test coverage. A few issues to address before merge:
Issues
1. Stale <param> in XML doc comment (line ~134)
RunIncrementalEdit has a <param name="unchangedSources"> tag in its XML doc, but the method signature is (initialSources, modifiedPath, modifiedSource) — there is no unchangedSources parameter. This is a leftover from an earlier design. Remove the stale <param> line.
2. GetAllTrackedStepNames is dead code
GetAllTrackedStepNames is defined in SourceGeneratorTestContext.cs but never called anywhere in the test suite. Either add a test that uses it (e.g. a diagnostic helper that dumps all tracked names on failure), or remove it to keep the helper surface minimal.
3. Tests don't assert that unmodified-file transforms are Cached
The 3 edit tests assert that the Combine stage is Modified/New and that at least one transform output is Modified/New. But the core value of incremental editing is that unchanged inputs stay Cached. Consider adding an assertion that at least one transform output for the unmodified file is Cached (or that the count of Cached outputs is >= 1). Without this, the tests would still pass even if the pipeline incorrectly re-ran all transforms.
For RegisterStrategy_EditOneFile_UnmodifiedContractStaysCached — the test name promises "UnmodifiedContractStaysCached" but doesn't actually verify caching of the unmodified contract. This is misleading.
Nits
- The
using Microsoft.CodeAnalysis;import was added at the top ofIncrementalCacheTests.cs— verify it's actually needed (forIncrementalStepRunReason). It's fine since that's where the type lives. - The box-drawing comment separator is fine but unusual in this codebase — check that
.editorconfigdoesn't flag non-ASCII comment characters.
What's good
RunIncrementalEditcorrectly usescompilation.ReplaceSyntaxTree(notWithSyntaxTreeswhich doesn't exist onCSharpCompilation).disabledOutputs: IncrementalGeneratorOutputKind.None+trackIncrementalGeneratorSteps: trueis the right configuration for cache testing.- The 3 tests cover the 3 distinct pipeline shapes: Collect+Combine (Strategy), per-item output (Singleton), and Collect+Combine with single stream (StateTransition).
- Test method names are descriptive and follow the
Generator_Scenario_ExpectedBehaviorconvention.
- Remove stale <param name='unchangedSources'> from RunIncrementalEdit XML doc comment (the parameter does not exist in the method signature). - Remove dead code GetAllTrackedStepNames (defined but never called). - Add Cached/Unchanged assertions to GenerateSingleton and StateTransition edit tests: the unmodified file's transform should be Cached or Unchanged, verifying the core incremental guarantee. - Rename RegisterStrategy_EditOneFile_UnmodifiedContractStaysCached to RegisterStrategy_EditOneFile_CombineReflectsChange: the unmodified file's transform is Modified (not Cached) because Transform returns List<KeyedRegistration> (reference equality). This will be fixed by PR #216 (LocationInfo + EquatableArray), but until then the test correctly verifies only the Combine stage behavior. Added XML doc explaining the limitation.
Review issues fixed in latest commit
All 133 tests pass. |
Summary
RunIncrementalEdit<TGenerator>helper toSourceGeneratorTestContextthat runs a generator on an initial compilation, replaces one syntax tree with a modified version, and re-runs. This enables testing the core incremental value of source generators: editing one file should only regenerate affected contracts.GetTrackedStepReasonsandGetAllTrackedStepNameshelpers for inspecting tracked step reasons from aGeneratorDriverRunResult.IncrementalCacheTests:RegisterStrategy_EditOneFile_UnmodifiedContractStaysCached— two strategy contracts in separate files, add a strategy to one file, assertStrategyCombineisModified/Newand transform reflects the new strategy.GenerateSingleton_EditOneFile_TransformReflectsChange— two singleton targets in separate files, add a singleton to one file, assert transform has at least oneModified/Newoutput.StateTransition_EditOneFile_CollectIsModified— two state machines in separate files, add a transition to one file, assertStateMachineCombineisModified/Newand transform reflects the change.What these tests prove
The existing
AssertCacheHittests only verify "same compilation, second run → all stages Cached". The new tests verify the real incremental scenario: when a file is edited, the pipeline detects the change (Combine is Modified) and the transform stage produces new/modified outputs for the changed content. This is the primary performance benefit ofIIncrementalGeneratorin large projects.Design notes
StrategyCollectandStateMachineCollecttracking names are defined inTrackingNames.csbut not used in the generators (noWithTrackingNameonCollect()). The tests useStrategyCombine/StateMachineCombineinstead, which cover the Collect+Combine aggregation.GenerateSingletonGeneratorhas noCollectstage (per-item output), so the test checks the transform tracking name directly.Related Issue
Closes #213
Solution module
Type of change
Test plan
dotnet build DesignPatterns.slnx -c Release— 0 errors, 0 warningsdotnet test DesignPatterns.slnx -c Release --no-build— all 644 tests pass (641 existing + 3 new)Breaking changes