Repository navigation
fix(skill): correct TestDoubles matching guidance #113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5139507
fix(skill): correct TestDoubles matching guidance
ncipollina caae88c
fix(skill): repair shared double examples
ncipollina e68a5db
fix(skill): correct namespace in shared-double benchmark output
ncipollina 04d5d46
fix(skill): add old-skill baseline for testdoubles benchmark
ncipollina 77ead53
fix(skill): rerun testdoubles benchmark as a true paired batch
ncipollina c86a166
docs(skill): fix self-contradictory baseline wording in benchmark.md
ncipollina File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
113 changes: 113 additions & 0 deletions
113
docs/research/0012-aws-secrets-manager-provider-compono-skill-regression.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| # [RESEARCH-0012] AWS Secrets Manager Provider Compono Skill Regression | ||
|
|
||
| **Status:** Done | ||
|
|
||
| **Date:** 2026-08-27 | ||
|
|
||
| ## Scope | ||
|
|
||
| This records two focused follow-up findings from the real AWS Secrets Manager | ||
| Provider migration from AutoFixture/AutoFixture.Xunit3/NSubstitute to | ||
| `Compono.XunitV3` + `Compono.TestDoubles`. | ||
|
|
||
| This is not a product design record and creates no ADR. It applies the existing | ||
| ADR-0029/ADR-0042 evidence rules to classify what the migration actually showed. | ||
|
|
||
| ## Finding A: installed skill missed shipped argument matching support | ||
|
|
||
| During the migration, the installed Compono skill initially led the agent to | ||
| write hand-made recording fakes for `IAmazonSecretsManager`, | ||
| `IConfigurationBuilder`, and `ILoggerFactory`, asserting that | ||
| `Compono.TestDoubles` intentionally did not support argument matchers/capture. | ||
| That was incorrect for ordinary matching and filtered verification. | ||
|
|
||
| The final migration used the shipped `Compono.TestDoubles` surface directly: | ||
|
|
||
| ```csharp | ||
| configurationBuilder.Configure() | ||
| .Add(Match.Any<IConfigurationSource>()) | ||
| .Returns(configurationBuilder); | ||
|
|
||
| configurationBuilder.Verify() | ||
| .Add(Match.Is<IConfigurationSource>(predicate)) | ||
| .Once(); | ||
| ``` | ||
|
|
||
| and: | ||
|
|
||
| ```csharp | ||
| secretsManager.Configure() | ||
| .GetSecretValueAsync( | ||
| Match.Is<GetSecretValueRequest>(predicate), | ||
| Match.Any<CancellationToken>()) | ||
| .Returns(Task.FromResult(response)); | ||
| ``` | ||
|
|
||
| The migrated suite passed across `net8.0`, `net9.0`, `net10.0`, and `net11.0` | ||
| (62/62 on each TFM), with AutoFixture/NSubstitute/`Compono.NSubstitute` absent | ||
| from direct and transitive dependencies. | ||
|
|
||
| Classification: **skill/docs regression**, not a runtime `Compono.TestDoubles` | ||
| capability gap. Current `Compono.TestDoubles` supports literal equality | ||
| matching, `Match.Any<T>()`, `Match.Is<T>(predicate)`, argument-filtered | ||
| `Never()`/`Once()`/`Exactly(n)`, and multi-entry argument-distinguished response | ||
| configuration for eligible member shapes. True capture/callback behavior remains | ||
| a separate boundary. | ||
|
|
||
| ## Finding B: `TestConfigurationProvider` is existing project-local test architecture | ||
|
|
||
| The migration retained a local `TestConfigurationProvider : ConfigurationProvider`. | ||
| This is not new evidence for abstract-class generated doubles. | ||
|
|
||
| Pre-migration evidence from the AWS Secrets Manager Provider test project: | ||
|
|
||
| ```csharp | ||
| public class ConfigurationProviderSpecimenBuilder : ISpecimenBuilder | ||
| { | ||
| public object Create(object request, ISpecimenContext context) | ||
| { | ||
| if (request is Type type && type == typeof(ConfigurationProvider)) | ||
| { | ||
| return new TestConfigurationProvider(); | ||
| } | ||
|
|
||
| return new NoSpecimen(); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ```csharp | ||
| public class TestConfigurationProvider : ConfigurationProvider | ||
| { | ||
| public override void Set(string key, string value) | ||
| { | ||
| Data[key] = value; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| The fixture registered that specimen builder before `AutoNSubstituteCustomization`: | ||
|
|
||
| ```csharp | ||
| fixture.Customizations.Add(new ConfigurationProviderSpecimenBuilder()); | ||
|
|
||
| fixture.Customize(new AutoNSubstituteCustomization | ||
| { | ||
| GenerateDelegates = true | ||
| }); | ||
| ``` | ||
|
|
||
| Search of the pre-migration state found no `Substitute.For<ConfigurationProvider>()` | ||
| and no NSubstitute-backed substitution of `ConfigurationProvider`. | ||
|
|
||
| Classification: **existing project-local test architecture**, not a Compono | ||
| capability gap. Compono preserves the same design through: | ||
|
|
||
| ```csharp | ||
| builder.Register<ConfigurationProvider>(_ => new TestConfigurationProvider()); | ||
| ``` | ||
|
|
||
| ADR-0042 Amendment 2 does not apply because there is no evidenced | ||
| `Compono.NSubstitute`/NSubstitute-can vs. `Compono.TestDoubles`-cannot | ||
| replacement case. No roadmap item, ADR, or abstract-class support design is | ||
| created from this evidence. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| { | ||
| "skill_name": "compono", | ||
| "date": "2026-08-27", | ||
| "runner": "pi --print --no-context-files --no-skills --skill skills/compono --thinking minimal", | ||
| "scope": "Focused AWS Secrets Manager Provider TestDoubles matching/capture regression evals only", | ||
| "results": [ | ||
| { | ||
| "eval_id": 28, | ||
| "name": "aws-iconfigurationbuilder-argument-filtered-verification", | ||
| "configuration": "with_skill", | ||
| "passed": true, | ||
| "evidence": "Output uses Configure().Add(Match.Any<IConfigurationSource>()).Returns(configurationBuilder) and Verify().Add(Match.Is<IConfigurationSource>(...)).Once(), maps Arg.Is to Match.Is, and does not recommend a recording fake." | ||
| }, | ||
| { | ||
| "eval_id": 29, | ||
| "name": "aws-iamazonsecretsmanager-argument-matched-configuration", | ||
| "configuration": "with_skill", | ||
| "passed": true, | ||
| "evidence": "Output uses Configure().GetSecretValueAsync(Match.Is<GetSecretValueRequest>(...), Match.Any<CancellationToken>()).Returns(...) and says no fake/NSubstitute is needed for the eligible member." | ||
| }, | ||
| { | ||
| "eval_id": 30, | ||
| "name": "adversarial-nsubstitute-vocabulary-trap", | ||
| "configuration": "with_skill", | ||
| "passed": true, | ||
| "evidence": "Output maps Arg.Is/Arg.Any/Received/DidNotReceive to Match.Is/Match.Any/Verify().Once/Exactly/Never and rejects recording fakes solely for NSubstitute vocabulary." | ||
| }, | ||
| { | ||
| "eval_id": 31, | ||
| "name": "true-callback-capture-boundary", | ||
| "configuration": "with_skill", | ||
| "passed": true, | ||
| "evidence": "Output says ordinary matching/filtering is supported but invocation-aware callbacks/capture/delegate invocation are outside Match<T>; recommends local fake or NSubstitute seam." | ||
| } | ||
| ], | ||
| "summary": { | ||
| "passed": 4, | ||
| "total": 4, | ||
| "pass_rate": 1.0, | ||
| "old_eval_regression_status": "not_rerun_full_benchmark_claude_weekly_limit; existing eval definitions 1-27 unchanged" | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Compono skill regression benchmark — 2026-08-27 | ||
|
|
||
| Focused regression run for the AWS Secrets Manager Provider TestDoubles matching/capture skill fix. | ||
|
|
||
| ## Scope | ||
|
|
||
| New evals added in `skills/compono-evals/evals.json`: | ||
|
|
||
| - 28 — `IConfigurationBuilder` argument-filtered verification | ||
| - 29 — `IAmazonSecretsManager` argument-matched configuration | ||
| - 30 — adversarial NSubstitute vocabulary trap | ||
| - 31 — true callback/capture boundary | ||
|
|
||
| ## Runner | ||
|
|
||
| Executed with Pi print mode against the repo skill source: | ||
|
|
||
| ```bash | ||
| pi --print --no-context-files --no-skills --skill skills/compono --thinking minimal "<eval prompt>" | ||
| ``` | ||
|
|
||
| Claude Code was attempted first but was unavailable because the local account had hit its weekly limit. The attempted outputs contained only the limit message and were discarded. | ||
|
|
||
| ## Results | ||
|
|
||
| | Eval | Result | Evidence | | ||
| |---|---:|---| | ||
| | 28 | Pass | Output uses `configurationBuilder.Configure().Add(Match.Any<IConfigurationSource>()).Returns(configurationBuilder)` and `configurationBuilder.Verify().Add(Match.Is<IConfigurationSource>(...)).Once()`; maps `Arg.Is` to `Match.Is`; no fake recommended. | | ||
| | 29 | Pass | Output uses `secretsManager.Configure().GetSecretValueAsync(Match.Is<GetSecretValueRequest>(...), Match.Any<CancellationToken>()).Returns(...)`; explicitly says no NSubstitute substitute or hand-written fake is needed. | | ||
| | 30 | Pass | Output maps `Arg.Is`/`Arg.Any`/`Received(1)`/`Received(2)`/`DidNotReceive` to `Match.Is`/`Match.Any`/`Verify().Once()`/`Exactly(2)`/`Never()` and rejects recording fakes solely for NSubstitute vocabulary. | | ||
| | 31 | Pass | Output distinguishes ordinary matching/filtering from invocation-aware callback/capture behavior and recommends a project-local fake or NSubstitute seam for the unsupported callback boundary. | | ||
|
|
||
| Focused new-regression pass rate: **4/4 (100%)**. | ||
|
|
||
| ## Existing eval behavior | ||
|
|
||
| The existing eval definitions (1–27) were left unchanged; only evals 28–31 were appended. A full historical with/without-skill benchmark could not be rerun in this environment because Claude Code returned the local weekly-limit message. No comparable old-eval regression benchmark is recorded here. |
54 changes: 54 additions & 0 deletions
54
skills/compono-evals/benchmarks/2026-08-27/outputs/eval-28-with-skill.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| Use a generated `IConfigurationBuilder` double as a shared parameter, configure `Add(...)` to return the builder, then verify with `Match.Is<T>(...)`: | ||
|
|
||
| ```csharp | ||
| using Compono.TestDoubles; | ||
|
ncipollina marked this conversation as resolved.
Outdated
|
||
| using Compono.XunitV3; | ||
| using Microsoft.Extensions.Configuration; | ||
| using Xunit; | ||
|
|
||
| public sealed class SecretsManagerConfigurationBuilderExtensionsTests | ||
| { | ||
| [Theory] | ||
| [Compose<GeneratedTestDoubleProfile>] | ||
| public void AddSecretsManager_adds_secrets_manager_configuration_source( | ||
| [Shared] IConfigurationBuilder configurationBuilder) | ||
| { | ||
| configurationBuilder.Configure() | ||
| .Add(Match.Any<IConfigurationSource>()) | ||
| .Returns(configurationBuilder); | ||
|
|
||
| configurationBuilder.AddSecretsManager(); | ||
|
|
||
| configurationBuilder.Verify() | ||
| .Add(Match.Is<IConfigurationSource>( | ||
| source => source is SecretsManagerConfigurationSource)) | ||
| .Once(); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Key migration points: | ||
|
|
||
| ```csharp | ||
| Arg.Is<IConfigurationSource>(...) | ||
| ``` | ||
|
|
||
| becomes: | ||
|
|
||
| ```csharp | ||
| Match.Is<IConfigurationSource>(...) | ||
| ``` | ||
|
|
||
| and: | ||
|
|
||
| ```csharp | ||
| configurationBuilder.Received(1).Add(...) | ||
| ``` | ||
|
|
||
| becomes: | ||
|
|
||
| ```csharp | ||
| configurationBuilder.Verify().Add(...).Once(); | ||
| ``` | ||
|
|
||
| Because `IConfigurationBuilder.Add` returns non-nullable `IConfigurationBuilder`, configuring it to return `configurationBuilder` avoids an unconfigured generated-double return failure and preserves the fluent builder behavior. | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.