Repository navigation
Added nonsentiel default tests - #54
Merged
Merged
Conversation
DriverRunResult is IDisposable: for a CompileToAssembly run, Dispose unloads the collectible AssemblyLoadContext holding the emitted assembly and releases the MetadataLoadContext. None of the 75 .Compile() call sites honoured that contract, so every emitted assembly stayed loaded for the lifetime of the test run. Adds `using` to the 71 direct call sites. The remaining four call BuildEnumDefinitionAsync, which returns a non-disposable JsonSchemaDefinition -- the leak was inside the helper, so the result is disposed there instead, after the schema has been built. This is a correctness fix against the documented contract, not a memory fix: peak working set across the four TFM processes is unchanged (measured 15.5 GB vs 14.0 GB baseline, within run-to-run noise). The dominant cost is per-test MetadataReference re-creation in the Testing harness, which is being addressed upstream in Purview.SourceGeneratorFramework.
Add Purview.Build 0.5.2 to .config/dotnet-tools.json and replace the hand-rolled dotnet tool install --tool-path .tools/purview-build in the Justfile with dotnet tool restore, running the pipeline as dotnet purview-build. The version is now pinned in the tool manifest alongside CSharpier, so it is restored by the same command as every other tool and needs no bespoke install step, no --add-source feed argument and no platform-specific executable suffix. The dead pipeline_feed variable is removed, and the docs describing the old .tools/purview-build path are updated to match.
Purview.Build 0.5.3 defaults RequiredCompilerFlags to empty. Earlier versions forced the optimization=release check on and could not be configured off, because binding an empty JSON array contributes no configuration keys and so never overwrote the tool's non-empty code default. These packages have therefore always been validated against that flag. Declaring it keeps that behaviour rather than silently losing the check on the move to 0.5.3. Verified with just pipeline-pack-validate: all packages still valid.
Picks up the fix that makes RequiredCompilerFlags configurable: earlier versions forced the compiler-flag check on because binding an empty JSON array contributes no configuration keys, so the tool's non-empty code default could never be overridden. This repository declares "RequiredCompilerFlags": ["optimization=release"] explicitly, so the check it has always run under is preserved. Verified with just pipeline-pack-validate against 0.5.3.
The workflows pin the SDK independently of global.json, so bumping one without the other leaves CI installing an SDK the repository then refuses to use.
Purview.BuildSdk 1.0.6 removes skill packaging and the build-time .agents/ mirror, so the skills CLI cloning the git repository is the only delivery route. Skills are authored at skills/<name>/SKILL.md and nothing packs them. - rename every skill to a purview-<product>-* prefix; skills install into one flat directory per agent, so an unprefixed name collides with somebody else's and the loser is whichever installed second - validate them on every test run with Purview.Skills.Testing 1.0.6, through a Repository.UnitTests project whose [InheritsTests] attribute is load-bearing - raise the TUnit floor to 1.75.0, which Purview.Skills.Testing depends on and central transitive pinning would otherwise pin back down
A tracked solution-level setting, so format-on-save matches the lint check the pipeline runs rather than being set per developer.
Rename the CHANGELOG's `Unreleased` heading to `2.1.0`, consolidating the 2.0.1, 2.0.1.1 and 2.0.2 incremental releases under it so a merge to main cannot publish an unnamed release. Restructure the former "breaking changes" section as `Upgrade notes`, each item naming the old form, the new form and the edit to make. Record ZODSGEN045 under a new `## Release 2.1.0` block. The guard that was supposed to catch it being left unshipped was passing vacuously: the rule-row regex matched only the pipe-delimited form used by Shipped.md, while Unshipped.md is written without leading pipes, so a rule parked there parsed as no rules at all and the assertion compared two empty sets. Accept both table styles. Enable package validation against the 2.0.2 baseline so an unintended public API change fails `dotnet pack`. All four packages validate clean with no suppressions, confirming 2.1.0 is additive over the published 2.0.2. Also document the pre-merge release checklist and correct the target framework list in README.md.
The cross-platform job selects its C# fixture producers with a treenode filter and carried only one, `/*/ZodSharp.Json/*CrossPlatformTests/*`, which writes src/tests/cross-platform/output/. The parity artifacts tests/ts/guid-uuid-parity.test.ts reads come from a second producer added with the Guid/UUID export work, in a different assembly, namespace and class (ZodSharp.SourceGenerators/ZodSchemaGeneratorTests/ JsonSchema_GivenGeneratedGuidSchemas_WritesZodParityArtifacts), which that filter does not match. src/tests/cross-platform/ is gitignored, so CI starts empty, guid-parity/ was never written, and `bun run test` failed every run. Run a step per producer. They cannot be combined: `a|b` is not honoured as a top-level alternation by the treenode filter - it selects a broader set (264 tests) and still writes no parity artifacts, which was verified while fixing this. `just test` was unaffected, because it runs the whole suite and so covers both producers. Verified by reproducing the CI sequence from a clean src/tests/cross-platform: both producers now write their fixtures (8 output files, 3 parity artifacts) and `bun run test` passes 29/29. Document the two producer/consumer pairs in AGENTS.md, including that adding a producer means adding a step.
The TypeScript/Zod half of the cross-platform parity guarantee ran only in the dedicated `cross-platform` CI job, so a Zod-side regression could not fail a local `just test` - and a gap in that job's own wiring (a fixture producer it never ran) went unnoticed for a whole release. Repository.UnitTests now executes each tests/ts/*.test.ts suite as a child process through CliWrap, one test per suite, so a failure names the file and the child's stdout/stderr becomes the assertion message. This duplicates the CI job deliberately rather than replacing it: the job stays the authoritative gate because it controls producer ordering and installs bun from scratch. Three rules keep the duplication honest: - One test per suite, for attributable failures. - VitestSuites_GivenEveryTestFileOnDisk_AreAllCovered fails when a .test.ts file has no entry in Suites(), so a new suite cannot end up driven by the CI job alone - the same class of gap as the producer above. - Missing prerequisites skip rather than fail. bun and node_modules/ are absent on the shared pipeline's build runner and before `bun install`; reporting a missing toolchain as a broken repository would be wrong. The coverage guard needs neither, so it still runs there. src/tests/cross-platform/ is gitignored and the producers live in other assemblies the platform may still be running in parallel, so these tests wait for their fixture directory to fill before skipping, rather than nesting a second `dotnet test` inside the run. Verified all three paths, not just the green one: an injected parity violation fails the .NET test with the vitest output attached; an unlisted .test.ts file fails the coverage guard; and with bun off PATH the two suite tests skip with their reason while the guard still passes. Full suite 5476/0, pack validation clean.
A skipped test proves nothing, and the cross-platform parity guarantee is not optional - so the vitest driver no longer skips when its toolchain is missing. The earlier skip was also based on a wrong assumption: bun IS installed on the shared build pipeline's runner. Resolve a runner per run instead: - bun when it is on PATH (what CI uses). - otherwise node, against vitest's own entry point. - neither: the test fails and says to install one. node_modules/ is gitignored, so when vitest is missing the driver installs it first - `bun install --frozen-lockfile`, or `npm install` when only node is present. bun.lock is the only lockfile, so npm resolves from package.json, which is a second reason bun is preferred for both steps. An install or run failure fails the test with the captured output. The node path invokes node_modules/vitest/vitest.mjs (vitest's own package.json `bin` entry) rather than npx or node_modules/.bin. Both are unreliable: the .bin shims belong to whichever package manager installed the tree - a bun install leaves vitest.exe/vitest.bunx and no node shim - and on Windows npx resolves to a .cmd/.ps1 shim, which cannot be started directly as a child process. The PATH probe therefore accepts only a real executable image, not a shim. Verified every path: bun 3/3; node fallback 3/3 with bun off PATH, confirmed to be the node branch because an injected parity violation reports `vitest (node)`; and with both off PATH the two suite tests fail rather than skip, while the coverage guard still passes. Full pipeline 5476 tests, 0 failed, 0 skipped.
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.
No description provided.