Split the C# suite so a test cannot silently leave the run - #252
Open
alextomas955 wants to merge 58 commits into
Open
Split the C# suite so a test cannot silently leave the run#252alextomas955 wants to merge 58 commits into
alextomas955 wants to merge 58 commits into
Conversation
- Renamer.Cove.Tests holds tests that need a real CoveContext; its Cove.Data and Cove.Core references are unconditional and a PrepareForBuild Error states the requirement instead of letting an empty reference path drop out silently. - RootNamespace is pinned to Renamer.Tests: following the project name would create a Renamer.Cove namespace that shadows Cove.* for every file here. - InternalsVisibleTo names the new assembly from both Renamer and Renamer.Tests.
…de conditionals - The 65 tests that need a real CoveContext now live in Renamer.Cove.Tests, mirroring the same folder names; Renamer.Tests keeps the pure tier. - Renamer.Tests no longer reads $(CoveSourceMode) at all: the Compile Remove set, the mode-guarded shared-fake include, the two Cove ProjectReferences and the Cove.Plugins package reference are gone, and the EF and TestHost packages go with the tests that used them. - The drive-letter collection is declared in both assemblies; xUnit scopes a collection to one assembly and reports nothing when it is missing.
…rce mode With a Cove checkout, Cove.Sdk is a Private=false / ExcludeAssets=runtime ProjectReference, so nothing copied EF Core into the pure project's bin/ and 42 tests failed at load with FileNotFoundException. The compile resolves EF Core transitively in both modes, so no build check could see this.
…pairing - coveTestProjectPath joins matrixPathFields, so a declared path that does not exist is an error - the Cove test project is asserted present in CoveExtensions.slnx alongside the pure one - declaring coveTestProjectPath without testProjectPath is refused: the Cove tier reaches the shared TestSupport helpers through a ProjectReference onto the pure one
… leg - a second dotnet test step, fed coveTestProjectPath through the step environment and guarded on it being non-empty, so an entry with no Cove tier is skipped rather than failed - the bare-leg and job-header comments now state why each tier runs where it does
- csharp-format checks Cove out at the highest declared floor and runs both its steps in source mode, so the solution stays its whole subject list and --exclude ../cove/ resolves - windows-build-test builds each catalog entry's cove-absent test project instead of the solution, which now holds a project that mode cannot build
…d one - Renamer.Tests/README.md: both projects, what decides which one a test belongs in, what each CI leg proves, and the cross-assembly subst drive-letter constraint - testing.md: which project you named decides the set; the leg shapes updated to the two cove-present invocations and the two lint legs - configuration.md: the analyzer gate builds in source mode - Directory.Build.targets, CLAUDE.md, authoring-patterns.md, development.md, the extension README and the pull-request checklist - the three Contracts/ placement remarks are dropped: the folder they argued against is no longer treated specially
… checkout - Renamer.Cove.Tests declares RequiresCoveSource, so the root names no project - SkipWithoutCoveSource.targets overrides Build with a notice naming the skip - Directory.Build.targets imports it under a solution-build, non-source condition
…ack is refused - the Sonar note names the analyzer gate's source mode instead of one mode for all legs - the location gate's note states how a clean clone now succeeds - both refusal clauses give the reason that holds today
…s authority - the analyzer note ends on the claim it can make for itself - the format step's note states the property that survives the npm wrapper
…o not load - Hand-transcribed expectations for the real line, its repeat, two projects, empty input and a near miss
…ces for - Pure named export over the tool's combined output, deduplicated in first-appearance order - Recognises only the tool's own sentence shape, so a near miss yields no name
- Wrapper spawns dotnet format with an argument vector, no shell, and returns its status unchanged - Adds one PARTIAL line naming any project whose references did not load - Pre-commit hook and both repo-root C# format scripts route through it with their arguments intact
- The route table the doc pins is described by what it constrains, not by a label
…analyzer command - the green-build caveat now names the per-project skip line a none-mode solution build emits, rather than a reduced test set that no longer exists - the C# analyzer gate row carries the source-mode command the workflow's analyzer step declares, which runs green from the repo root
- the pre-commit list names the format script rather than the tool it wraps - a troubleshooting entry quotes the disclosure prefix the pass prints when a project's references do not load, with the checkout that restores coverage - corrected two claims the workflow contradicts: CI checks Cove out beside this repo, so the exclude is load-bearing there, and the job shares the subject set and arguments with the check script rather than the invocation
…pile items they are - the fakes are pulled in unconditionally rather than under a source-mode predicate the item group does not carry - names the checkout requirement as the project's own target, and the skip a solution build with no checkout prints instead of failing
The source-mode gate's error text names the skipped project as what a fallback would cost. The page gave a different reason, which the announced skip no longer supports.
… one - the skip-gate section restores the do-not-read-the-Windows-leg warning, scoped to the split: the gated cases in the Cove-dependent project run in no CI leg, and a reader is told to check which set they ran - the windows-build-test bullet names what that leg reaches instead of claiming it executes the Windows-gated assertions
The windows-build-test sentence now states what that leg reaches and what runs in no CI leg, in the clause the test project README carries, and repeats the instruction to check which set was actually run.
- the leading command pins module parallelism to one, so the page's own drive-letter warning is expressed in the command rather than left to the reader; the paragraph below says why - the checkout requirement states the real failure mode: the skipped project's test executable is missing, and the summary still reads Passed over the project that did build - the symptom list separates the direct build that stops from the solution run that exits non-zero under a green summary
… rationale Two passages still argued from a mechanism that no longer exists. The refusal reason now matches the gate's own error text and the wording the architecture page carries, and the none branch is described as it behaves: a solution build names each project it skipped.
…urce is absent - Split the skip condition into _CoveSourceMissing (the project cannot build in this mode) and _CoveSolutionBuildSkip (that, scoped to a solution build) - Set IsTestProject/IsTestingPlatformApplication/IsPublishable false under the former, so dotnet test does not enumerate the module and publish does not copy it - Remove the project's OutputPath in the overriding Build target, so a run that reaches it anyway finds no executable rather than a previous build's - Emit the skip sentence as a Warning beside the Message, so a partial build lands in the build summary's warning count
… builds Each workflow states its own mode in its own -p:CoveSourceMode argument, which is where a reader checks it.
…does The run exits 0 over the checkout-free project alone and names neither the project it skipped nor the fact that it skipped one, so the page no longer offers the exit code as the check, and points at dotnet build for the name. Carry the module-parallelism flag into the tier table, so the first command on the page is the one the C# section pins.
…ct itself on - invokedAsScript() and the typeof import.meta.main guard, copied from the two sibling CLI scripts so all three refuse the absent feature by name - the refusal is scoped to the entry, so importing the parse is unaffected
…ot be truncated away - the PARTIAL line's subject and pronouns agree for one project as well as several - maxBuffer raised, so a large finding set cannot truncate the line appended after it - two comments restated as constraints: the spawn relies on shell: false, not on the shell being absent from the staged file list
…e hook runs - the csharp-format job's verify step invokes the wrapper, so its partial-coverage disclosure exists where it can block a merge - working-directory and the COVE_SOURCE_MODE env block are unchanged
… agree - the analyzer gate row's checkout prerequisite states both routes and CI's own clone - the local-versus-CI sentence names the depth difference the page explains further down - the green-build bullet names the command each line appears on, and the one it does not
…low makes - the exclude clause carries the wording the development page already carries - the second reason, about a folder path needing a trailing separator, is unchanged
…e test project - A violating fixture whose extra project references the declared coveTestProjectPath - The same reference spelled with backslash separators - A non-violating reference onto a different project, with the scanned count asserted
…st project - Walks the repository for .csproj files and resolves each ProjectReference Include against the containing project, so a project the catalog does not declare is reached - Reports the scanned count and the number of declared Cove test projects, so a walk that found no subject cannot read as a pass
… pins - Drops the clause naming a refactor that has not happened and the paragraph describing a claim a reader has never seen - Demoted from an XML doc to a plain comment, which is the policy on test code
Node's stdout is asynchronous on a pipe and process.exit does not drain it, so the wrapper printed a truncated wall of findings and dropped the PARTIAL line while keeping the exit status. Measured on this platform: 8 MiB written then a sentinel line, piped - 131072 bytes delivered and the sentinel lost with process.exit(), 8388618 bytes and the sentinel intact with process.exitCode.
… left out
IsTestProject=false removed the Cove-dependent project from the module list dotnet
test enumerates, so a solution-wide run in none mode exited 0 over 735 of 1019 tests
and named nothing. The stale-assembly defect it was reaching for is held by the
RemoveDir in the Build override, not by that property.
Measured in a detached worktree with no ../cove sibling and COVE_REPO unset, over a
bin/ populated by a prior source-mode build:
dotnet test CoveExtensions.slnx -p:CoveSourceMode=none
before: exit 0, total 735, 0 mentions of Renamer.Cove.Tests, stale apphost intact
after: exit 1, total 735, 3 mentions, stale apphost removed
dotnet test --project Renamer.Cove.Tests.csproj -p:CoveSourceMode=none
before: No test projects were found., exit 1, no cause named
after: exit 1, 'requires a Cove source checkout' naming COVE_REPO
dotnet build CoveExtensions.slnx -p:CoveSourceMode=none exit 0, 1 warning
COVE_REPO=... dotnet test CoveExtensions.slnx exit 0, total 1019, failed 0
dotnet test passes MSBuild warnings through and drops messages, which is what lets
the existing skip Warning carry the reason into the test run's own output. CI runs
dotnet test --project only, never the solution, so no workflow leg changes.
…uild has Measured: dotnet test prints the Warning from this target and drops the Message beside it, and the RemoveDir does run on that path - a stale apphost planted by a prior source-mode build was gone after a none-mode solution-wide dotnet test.
…nts mounts
MapEndpoints registers twelve routes and the theory drove eight; /scan-rows,
/library-paths, /job-status/{jobId} and /orphaned-rules answered nothing while the
class comment claimed every registered route was covered. The four are added, and a
new fact compares the hand-transcribed list against the route table the booted host
actually exposes, so a thirteenth registration fails rather than joining the claim
in silence.
Measured: the class went from 11 tests to 16, all passing; dropping one entry from
the list fails EveryMountedRoute_IsDrivenByTheRouteTheory.
…checkout refusal Restoring IsTestProject made a direct dotnet test --project on the Cove-dependent project build first, so it now stops on that project's own named refusal instead of reporting 'No test projects were found.' with no cause. Measured with no ../cove sibling and COVE_REPO unset: exit 1, 'Renamer.Cove.Tests requires a Cove source checkout (CoveSourceMode is none)'. That is the arrival path a contributor following the extension README or the pull-request template takes.
… spelling The rule matched one double-quoted path per attribute, so three spellings MSBuild accepts walked past a blocking gate that then reported the file as scanned: a semicolon-delimited item list, a single-quoted attribute, and a property-expanded Include - the last being the spelling this repo's own test projects use. Semicolon segments are now split and either quote delimits. A segment carrying a property or a wildcard cannot be resolved without MSBuild's evaluation, so it is named on its own NOTICE line and counted in the summary rather than passing as clean. On this repository that discloses three such Includes.
…oot build files Two directions, both driven by new tests that fail without the change: A commented-out ProjectReference matched the pattern and failed a blocking gate on text MSBuild never reads - and this repository already carries comment blocks discussing ProjectReference wiring in prose. The walk collected only .csproj, so Directory.Build.props and Directory.Build.targets were never read. A root-level injection is the one shape that adds a reference to every project beneath it without editing any project file, and it was the one shape the rule could not see. Scanned count on this repository goes from 5 to 7.
…output The summary reported a scanned count over a walk that skips five directory names and every symlinked directory, and said neither - the concession lived in a comment, which is not output. Both are now named on the summary line. A directory the walk cannot open took the whole validator down with a readdirSync stack trace. It is reported by name and fails the run instead, because the rule has read nothing under it either way.
…al coverage The lint job's whole preamble checks Cove out so the analyzer half of dotnet format applies. When that produced a tree whose references would not load, the step degraded to a whitespace-only check over the Cove-dependent project, printed one PARTIAL line, and merged green. --fail-on-partial is the wrapper's own flag, filtered out of what dotnet format receives, and passed only by that job. Local runs and the pre-commit hook keep reporting without gating, because working with no checkout is supported. Measured with no checkout: exit 0 without the flag, exit 1 with it and a named refusal on stderr; dotnet format reported no unrecognized argument. Measured with COVE_SOURCE_MODE=source and COVE_REPO set: exit 0, no PARTIAL line.
…and is pinned The guard runs only where import.meta.main is absent - Node below 22.18 - and on the oldest half of that range import.meta.filename is undefined too, so its first line of work was path.resolve(undefined) and the contributor got an ERR_INVALID_ARG_TYPE stack trace instead of a sentence naming their Node version. It falls back to fileURLToPath(import.meta.url), which exists everywhere. The comparison is now exported as isSameFile(entry, self, platform) and pinned: a path reached through a symlink names the same file as the real one, two real files do not, drive-letter casing folds on win32 and not elsewhere, and an absent argv[1] is false rather than a throw. None of that had a test.
…nies its own coverage The member had two consecutive <summary> blocks - malformed doc XML the emitted documentation file resolved by keeping one and dropping the other with no diagnostic, since CS1591 is the only doc warning this repo touches. The dropped one told a reader WebApplicationFactory cannot mount extension routes and that there is therefore no HTTP-level coverage of them, which TransportHost and TransportSmokeTests refute. The host-contract half of it earns its place and moves into <remarks>: where the lambda parameters come from, and that ICurrentPrincipalAccessor is populated by the host's CurrentPrincipalMiddleware. Measured: the emitted Renamer.xml now carries one summary tag for this member and no occurrence of the retired claim.
"The only C# coverage" is a count of one that no gate holds, so a second such test makes it false with nothing reporting the drift. The rest of the comment - why the class lives in the Cove-dependent project - earns its place and stays, and its coverage claim is now the pinned one. The sibling finding on Directory.Build.targets, a count of properties wrapped around what would happen without them, was rewritten with the CR-02 fix.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ver closes An unclosed XML comment leaves its opener behind a single strip pass, so a ProjectReference after it read as live and failed a blocking gate on markup MSBuild never reaches. Comment spans are computed and a match inside one is skipped; a file whose comment never closes is named and left unjudged, and the summary line counts those files.
…which it read A .props or .targets reaches a project without that project declaring anything, so a scan keyed on two conventional file names missed both the targets file this repository imports and any per-extension one. The extension selects now, and the summary states which extensions were read beside the directories that were not walked.
The workflow carried its own three-part comparator beside a prerelease-aware one this module already exported. The export it lacked is the pick itself, which now parses every floor before reducing - a one-entry list never invokes a reduce callback, so validating inside one returned a lone unparseable floor unchecked.
…ts match its steps The Windows job's comment gave a reason the solution-build skip now handles and claimed a solution build the job no longer runs. The C# style job's budget predates the Cove clone and Release build it now performs.
|
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
2 participants
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.
What changed
The C# suite was one project whose contents were switched by
CoveSourceMode, using ahand-maintained 28-entry
Compile Removelist. A test could leave the run silently — the list wasedited by hand, and nothing checked it.
It is now two projects:
Renamer.Tests(no Cove source-only reference) andRenamer.Cove.Tests(which requires a
../covecheckout). Misplacing a test is a compile error, not something a gatedetects afterwards. The exclusion list is gone, not relocated —
Compile Removeentries went 28 → 0.Alongside that:
checkout happens to be on disk. All 11
dotnetinvocations across the four workflow files carry anexplicit
-p:CoveSourceMode=.catalog.jsongainedcoveTestProjectPath, andscripts/validate-extension-repo.mjsenforces theshape — including refusing a
ProjectReferenceonto a Cove test project, which is the soundnesscondition the solution-build skip depends on. A second extension inherits the split with no edit.
scripts/check-csharp-format.mjsrefuses a runtime it cannot detect itself on (matching the guardits two sibling scripts already carry), its partial-coverage disclosure can no longer be truncated
away, and the merge-gating
dotnet formatstep routes through it — so the disclosure now exists inthe environment that can block a merge, not only in the one that cannot.
../covesibling now agree, andagree with what
lint.ymlactually does.No user-facing behavior changes, so there is no CHANGELOG entry.
How it was verified
Renamer:
dotnet test extensions/Renamer/src/Renamer.Tests/Renamer.Tests.csprojpasses — 735 total, 0 faileddotnet test extensions/Renamer/src/Renamer.Cove.Tests/Renamer.Cove.Tests.csprojpasses (with the../covesibling) — 289 total, 0 failedcd extensions/Renamer/src/Renamer.Ui && npm run verifypasses — typecheck, format, check-classes, check-host-imports, 203 testsAlways:
dotnet build CoveExtensions.slnxsucceeds — 0 Warning(s), 0 Error(s)testing.md,development.md,configuration.md,architecture.md,authoring-patterns.md,CONTRIBUTING.md, both READMEs and the PR templateAlso run:
npm test(repo tooling) 115 passed, andnode scripts/validate-extension-repo.mjsexit 0.
The counts partition exactly:
735 + 289 = 1024, the whole-solution total.The compile refusal was induced rather than assumed. A single file with
using Cove.Data;was builtin each project with the checkout present in both, so the difference is the project boundary and
not a missing sibling:
Safety check
Renamer file/DB behavior is untouched by this PR — no planner, executor, mover, or journal code
changed. The two boxes below are therefore unaffected rather than re-verified:
Notes for reviewers
The interesting bug was one this PR introduced and then removed. Making the skipped project
un-runnable was first done by setting
IsTestProject=false. That did stop a stale assembly beingrun — but it also dropped the project from enumeration, so a checkout-free
dotnet test CoveExtensions.slnx -p:CoveSourceMode=noneexited 0 while silently omitting 284tests. Measured across three commits, over a
bin/populated by a prior source-mode build:The property was removed; the
RemoveDirin the same change is what actually removes the staleassembly, and it is kept. Both halves now hold: no stale assembly runs, and the omission is loud.
Two things a reviewer should know:
dotnet testpasses MSBuild warnings through and drops messages. The skip notice is aWarningfor that reason.dotnet publish CoveExtensions.slnx -p:CoveSourceMode=nonestill exits 0 without naming theskipped project (
IsPublishable=falseremoves it from the graph, so theBuildoverride neverruns). No CI leg publishes the solution, so this is recorded rather than fixed.
Left out of scope: three Info-tier review findings, and the deferred phases 127 and 128.