Add agent skills for suite setup and test authoring - #13
Merged
Conversation
The example suites are the reference consumers copy from, so mistakes in them
propagate. Five surfaced while auditing them:
- UserIntegrationTests called TestReader.GetTestsForAFile with its arguments
reversed. The signature is (testFolderName, fileName); the other two suites
call it correctly. Harmless today only because the method is unreferenced.
- AuthTokenProvider implemented IAuthTokenProvider but was never referenced. No
SuiteBootstrapper overload accepts a custom provider — auth is configured
declaratively — so the class advertised an extension point that is not wired.
- The AppLauncher config described a Startup.SeedDatabase method that does not
exist. The real mechanism is the IsLocalComponentTests flag selecting an
in-memory database, with UserDbContext creating the schema once per process.
- appsettings.Tests.json carried an environmentVariables key nothing reads.
TestSuiteInitializer loads the file as a config source; a nested JSON object is
never applied as process environment variables.
- UserDbInitializer.Seed had a commented-out body, demonstrating nothing. It now
seeds reference data, showing where the onStarted hook belongs.
Also aligns the AppLauncher namespace with its folder, and takes the qa
environment URL from ${QA_API_URL} instead of a hardcoded host, matching the
secrets-from-environment rule used everywhere else.
EnableMockServerLogs existed on SuiteConfig but had no config-file equivalent, so turning it on meant editing fixture code. It is now a mock.enableLogs key, defaulting to false. Beyond debugging a stub that will not match, this makes an unknown dependency surface discoverable. Run a component test with no mock: block and every outbound call comes back 404 and is logged, with method, path, query and body — enough to write the interactions from. Because it observes HTTP rather than code, it works whatever language the service under test is written in, which is the practical way to build mocks for an AppLauncher suite. One caveat is documented alongside it: during that loop the test's pass/fail is meaningless. Unmatched 404s make the service take its dependency-failure path, which is often the very error the test expected, so a test can pass with every mock missing. Read the log, not the verdict.
example/ had no README, so a reader arriving from the docs had to infer which of the three projects demonstrated which startup mode, and which parts of them were ConfIT and which were the sample User API. Adds a mode-to-project map, the four things every suite structurally needs, and two things that look required but are not — TestOutputLogger is optional, proven by the AppLauncher suite passing null, and appsettings.Tests.json only applies to in-process mode. The section that earns its keep is "what is demo-specific": UserDbInitializer, JustAnotherService, the port numbers, the IsLocalComponentTests flag and the User.* namespaces, each with the reason it exists. Those are the parts a reader copies by accident.
Two scripts, both runnable without a build and both exiting non-zero on error, so
either can gate CI. A new `make skills` target runs them against every example
suite and is wired into `make ci`.
check-testcases.py reports what ConfIT would otherwise raise at load time or on
the first failing assertion: a response with no expected body, depends: naming an
unknown or later test, unresolvable {{variables}}, bodyFromFile pointing at a
missing file, mock: blocks in an integration suite, unregistered test files, and
matcher problems — an unknown name, a missing closing parenthesis, or a wildcard
or array index in a semantic path, none of which resolve.
Both read ground truth from src/ConfIT/ rather than hardcoding it: supported
target frameworks come from ConfIT.csproj, and built-in matcher names from
SemanticMatcher.cs, with names registered as custom matchers in the project's own
C# also accepted. A matcher added upstream therefore never leaves the checker
rejecting valid tests.
They live in tools/ beside push_nuget.sh rather than inside a skill, since the two
authoring skills share the first one.
Three skills that teach a coding agent to use ConfIT, split by the job being done
rather than by the feature being used.
confit-suite-setup wires a suite: startup mode, suite.config.yaml, fixture, test
class, auth and filters.
Authoring splits by persona, because these are different jobs even though they
produce nearly the same artifact — the same lifecycle scenario is 89 lines as a
component test and 64 as an integration test, differing only by a mock: block:
- confit-component-tests is the developer mid-implementation. Two inputs: the
controller, and the mocks behind it. It goes down the call path only as far as
the outbound calls, which is the one thing a spec can never reveal, and which
also surfaces the branch points that make error tests writable — a 400 driven by
changing what a mocked dependency returns, not by changing the request.
- confit-integration-tests is QA post-deployment. Black box, from a spec, a
Postman collection or a live probe, assuming no access to the service source and
possibly a different repository. It covers environment selection, auth profiles,
and the shared-persistent-state problem that makes suites pass once and fail on
the second run.
No skill carries templates. Each resolves the repository it ships inside and reads
example/ and doc/ directly, so what an agent is shown is what CI verifies. The
resolver walks up from the script's physical location looking for example/, doc/
and skills/ as siblings, which is symlink-safe and works from a plugin cache, a
clone or a local --plugin-dir load. When it cannot resolve, the skill stops rather
than inventing a suite from memory: a confidently stale sample is worse than none.
The skills are honest about what the library cannot do — no cleanup hooks, no
retry for eventual consistency, no request timeouts, no unique-data helpers — and
give the workarounds that do work, such as ${RUN_ID} interpolation for per-run
unique data and a trailing delete test for teardown.
Distribution is plugin-only. Installing a plugin clones the whole repository into the agent's cache, not just skills/, which is what lets each skill read the live example/ and doc/ instead of carrying copies that go stale. .claude-plugin/ makes the repository its own marketplace, so `/plugin marketplace add techygarg/ConfIT` then `/plugin install confit@confit` is the whole install. .codex-plugin/ carries the equivalent manifest pointing at the same skills/ folder. Onboarding another agent means adding one manifest directory at the repository root; nothing about the skills changes. skills/ must therefore stay at the root — both manifests name ./skills/, and the reference resolver expects example/, doc/ and skills/ to be siblings.
Adds doc/ai-skills.md covering the three skills, plugin install for Claude Code and Codex, and the reasoning behind the shape: why authoring splits by persona, and why plugins rather than an install script. The rationale worth keeping: example/ took 21 commits in twelve months, and the files a template would mirror churned hardest — one .csproj changed 13 times, and the fixture's shape changed materially when SuiteBootstrapper was introduced. A stale sample is worse than none, because an agent trusts it over the repository. Records the maintenance rules in CLAUDE.md: no skill may gain template files, new patterns go into example/ where CI runs them, and the two authoring skills are not to be merged back into one.
PR Summary by QodoAdd agent skills for suite setup and test authoring
AI Description
Diagram
High-Level Assessment
Files changed (40)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+51
to
+53
| public void LoadComponent_MockEnableLogsTrue_ParsesFlag() | ||
| { | ||
| var path = Write(""" |
There was a problem hiding this comment.
1. Parsing tests lack phase structure 📘 Rule violation ⚙ Maintainability
The new LoadComponent parsing tests are non-trivial but do not separate and label their setup, action, and assertion phases with Given, When, and Then comments. This violates the required test structure and reduces test readability.
Agent Prompt
## Issue description
The new non-trivial parsing tests do not clearly separate and label their setup, action, and assertion phases.
## Issue Context
Compliance rule 897534 requires non-trivial tests to use distinct `Given`, `When`, and `Then` blocks with blank-line separation. Preserve the existing `try`/`finally` cleanup behavior while restructuring the test bodies.
## Fix Focus Areas
- test/ConfIT.UnitTest/Config/SuiteConfigurationTests.cs[51-98]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| import re | ||
| import sys | ||
|
|
||
| # --------------------------------------------------------------------------- YAML subset |
There was a problem hiding this comment.
2. Python uses decorative banners 📘 Rule violation ⚙ Maintainability
check-testcases.py introduces repeated-punctuation comments as visual section dividers. The file therefore violates the prohibition on decorative comment banners.
Agent Prompt
## Issue description
The Python validator uses long hyphen-based comment banners as visual section dividers.
## Issue Context
Compliance rule 897522 prohibits decorative comment banners. Remove the repeated punctuation and rely on the file's functions and classes for organization, or replace each banner with a concise prose comment where clarification is necessary.
## Fix Focus Areas
- tools/check-testcases.py[30-30]
- tools/check-testcases.py[264-264]
- tools/check-testcases.py[298-298]
- tools/check-testcases.py[335-335]
- tools/check-testcases.py[369-369]
- tools/check-testcases.py[597-597]
- tools/check-testcases.py[640-640]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| echo "ConfIT suite check: $(basename "$CSPROJ")" | ||
|
|
||
| # --------------------------------------------------------------------- project references |
There was a problem hiding this comment.
3. Shell uses decorative banners 📘 Rule violation ⚙ Maintainability
verify-suite.sh introduces repeated-punctuation comments as visual section dividers even though its section() helper already labels those sections. The file therefore violates the prohibition on decorative comment banners.
Agent Prompt
## Issue description
The shell validator uses long hyphen-based comments as visual section dividers.
## Issue Context
Compliance rule 897522 prohibits decorative comment banners. The following banners are redundant because each is immediately followed by a call to the existing `section()` helper.
## Fix Focus Areas
- tools/verify-suite.sh[65-65]
- tools/verify-suite.sh[99-99]
- tools/verify-suite.sh[162-162]
- tools/verify-suite.sh[185-185]
- tools/verify-suite.sh[207-207]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+42
to
+46
| REGISTERED="$(tr '<' '\n' < "$CSPROJ" | awk ' | ||
| /^None[[:space:]]+Update=/ { p = (match($0, /Update="[^"]*"/) ? substr($0, RSTART + 8, RLENGTH - 9) : ""); next } | ||
| /^CopyToOutputDirectory>/ { if (p != "") { print p; p = "" } next } | ||
| /^\/None>/ { p = "" } | ||
| ' | tr '\\' '/')" |
There was a problem hiding this comment.
9. Indented registrations never parse 🐞 Bug ≡ Correctness
verify-suite.sh anchors its XML parsing at column zero, but the example .csproj <None> and <CopyToOutputDirectory> tags are indented. REGISTERED is therefore empty and make skills reports every runtime file as unregistered, causing the new CI prerequisite to fail.
Agent Prompt
## Issue description
`verify-suite.sh` does not recognize indented `<None Update>` registrations, so its registration check always fails for the repository's example projects and breaks `make skills`.
## Issue Context
The parser splits XML tags onto lines but uses patterns anchored directly at `None`, `CopyToOutputDirectory`, and `/None`. Project files conventionally indent those tags.
## Fix Focus Areas
- tools/verify-suite.sh[42-46]
- example/User.ComponentTests/User.ComponentTests.csproj[30-61]
- Makefile[66-75]
Update the parser to accept leading whitespace, then verify `make skills` succeeds on every listed example suite.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
ForComponent built the in-process TestHttpClient without ever calling cfg.ToAuthTokenProvider(), unlike ForCommand and ForIntegration. An auth: block under a component: section with startup.mode: in-process was parsed and validated but silently never attached to outbound requests. TestHttpClient and TestSuiteInitializer already accepted an optional IAuthTokenProvider; this just threads it through the one call site that was missing it, so declarative auth now behaves the same in every mode.
Two gaps let malformed input pass silently:
- discover() required every top-level value in a file to already look like
a test (a dict with an 'api' key) before reporting anything on it at all.
A malformed entry such as 'TestA: {}' made the whole file invisible to
the checker instead of flagging the entry — the file was treated like an
unrelated fixture, not a broken test. TestReader loads it regardless and
fails at runtime. Now: a directory with at least one properly-shaped test
file is a test directory, and any dict-shaped sibling there that isn't
correctly shaped gets reported instead of skipped. A suite.config.yaml
that declares a section but has zero discovered test files is now an
error too, instead of a silent pass.
- custom_matchers() whitelisted any '["key"] = ' string literal found in
any .cs file in the project, regardless of what dictionary it belonged
to. A typo'd or unregistered matcher name could coincidentally match an
unrelated dictionary key elsewhere in the codebase and pass validation,
then fail at runtime with an unknown-matcher exception. Detection is now
scoped to keys inside a 'Dictionary<string, SemanticMatcherFunc>'
initializer specifically.
Both checks grepped the whole config file instead of the block they were meant to validate: - The api.url check accepted any indented 'url:' anywhere in the file, so a component config with only mock.url (and no api.url at all) passed — SuiteConfiguration requires component.api.url unconditionally. - The readiness check accepted any indented 'port:' or 'url:' anywhere in the file — satisfied by the required api.url alone — instead of requiring exactly one of the two inside the readiness: block itself. An empty readiness block, or one with both port and url, passed even though SuiteConfiguration.ValidateComponent requires exactly one. Adds yaml_block(), which extracts the lines nested under a given key by indentation, and points both checks at the block they actually mean to inspect.
The component-tests skill stated as fact that state is fresh each run and fixed literal test data is safe, attributing it to "the in-memory database" resetting per process. That is true of ConfIT's own example, but it is the application's behavior, not something ConfIT provides — a command-mode suite (ForCommand/AppLauncher) may point at a real, persistent store, where the same literal-data habit produces the same collide-on-second-run bug the integration-tests skill already warns about. Reframes both bullets as conditional on confirming the app's own DB setup, and points to the integration skill's run-unique-data approach for when freshness isn't confirmed.
The documented trailing-cleanup test interpolated {{orderId}} without
depending on the test that extracts it. If the create test fails,
VariableInjector throws on the unresolved variable before the DELETE
request fires, turning the intended harmless cleanup into a second
failing test that obscures the original one — the opposite of what the
surrounding caveat claimed would happen.
Adds depends: CreateOrder, matching the skip-on-failed-prerequisite
behavior BaseTest already provides, and corrects the caveat to describe
that behavior instead of an assumed no-op.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
confit-suite-setup,confit-component-tests,confit-integration-tests— plus sharedtools/check-testcases.pyandtools/verify-suite.shvalidators gated bymake skills(wired intomake ci).claude-plugin/,.codex-plugin/), documented indoc/ai-skills.mdand the changelogexample/(reversedTestReaderargs, unusedAuthTokenProvider, staleAppLauncherseeding docs, deadappsettings.Tests.jsonkey, no-opUserDbInitializer.Seed) and addsmock.enableLogs(suite.config.yaml) plusexample/README.mdTest plan
make ci(build + unit + component + skills + integration)make skillsvalidates the tools/scripts against every example suiteclaude --plugin-dir .) and exercise each skill againstexample/