镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Set Verify solution context for project-only builds - #7488

Merged
thomhurst merged 1 commit into
mainfrom
issue-7419-verify-context
Nov 3, 2026
Merged

thomhurst merged 1 commit into
mainfrom
issue-7419-verify-context

Conversation

@thomhurst

@thomhurst thomhurst commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

Project-only snapshot test builds cannot choose between the repository’s three solution files. Default SolutionDir to the repository root and SolutionName to TUnit in shared build properties, preserving values supplied by solution builds or the command line.

Closes #7419

Validation:

  • Reproduced Verify’s ambiguity warning with DiscoverSolutionInfo before the change; the same target runs without warnings afterward.
  • Verified default properties, command-line overrides, and generated Verify assembly metadata.
  • Explicit single-node net10.0 build passed; core generator snapshot suite: 249 passed, 1 skipped, no snapshot changes.
  • git diff --check passed.

The initial parallel build encountered MSB4166 (already tracked in #7468). Validation used dotnet build ... -f net10.0 -m:1 -nr:false --no-restore followed by dotnet test --project tests/TUnit.Core.SourceGenerator.Tests --framework net10.0 --no-build. Existing dependency warnings are tracked in #7412.

Summary by CodeRabbit

  • Bug Fixes
    • Build configuration now uses sensible defaults when solution directory or name values are missing or undefined, improving consistency across builds.
    • Values already supplied by the solution or command line remain unchanged.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The shared build properties now provide defaults for SolutionDir and SolutionName when either value is empty or *Undefined*. Existing values remain unchanged.

Changes: Build property defaults

Layer / File(s) Summary
Set solution context defaults
Directory.Build.props
When SolutionDir or SolutionName is empty or *Undefined*, the build properties set defaults to the props file directory and TUnit, respectively. Existing values remain unchanged.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other · Severity of issue fixed: Low

Suggested reviewers: quattordici

Merge Risk: Minimal · up to c0117

No actionable merge-blocking risk is evident in the reviewed change.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Set Verify solution context for project-only builds" matches the main change. Directory.Build.props now supplies default SolutionDir and SolutionName values for Verify when they are empty o…
Linked Issues check Passed Issue #7419 asks for explicit Verify solution context so that routine builds of tests/TUnit.Core.SourceGenerator.Tests do not warn that Verify cannot choose among TUnit.CI.slnx, TUnit.Dev.slnx, and TU…
Out of Scope Changes check Passed The whole-PR diff contains one changed file, Directory.Build.props, with a 7-line addition and no other hunks. The change sets Verify solution properties only, and a comment explains the purpose. Each…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Title check

Explanation

The title "Set Verify solution context for project-only builds" matches the main change. Directory.Build.props now supplies default SolutionDir and SolutionName values for Verify when they are empty or Undefined, and existing values stay in place. The title names the change and its purpose, and it fits the linked issue about project-only builds of the source generator tests.

Full details: Linked Issues check

Explanation

Issue #7419 asks for explicit Verify solution context so that routine builds of tests/TUnit.Core.SourceGenerator.Tests do not warn that Verify cannot choose among TUnit.CI.slnx, TUnit.Dev.slnx, and TUnit.slnx. The head diff against the merge base changes only Directory.Build.props. It adds a PropertyGroup that sets SolutionDir to $(MSBuildThisFileDirectory) and SolutionName to TUnit when each value is empty or Undefined. Both settings are conditional, so values from solution builds or the command line are kept. This directly provides the configuration the issue requests. The PR validation notes report that the warning was reproduced before the change and absent afterward. I did not rerun the build. The pass rests on the code change, which implements the requirement. No coding requirement from the issue is unmet.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff contains one changed file, Directory.Build.props, with a 7-line addition and no other hunks. The change sets Verify solution properties only, and a comment explains the purpose. Each added line relates to issue #7419. The properties are defined in a shared props file, so they apply to all projects under the repository root. This is consistent with the issue and the PR description. No unrelated change was found.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Finishing Touches
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit checks the build with care,
Two defaults wait when values aren’t there.
The solution path, the name set right,
Existing choices stay in sight.
The tests can hop along their way.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 10b6ce52-d062-4850-8c4f-872933924000
Commits

Reviewing files that changed from the base of the PR and between 9100521 and c011729.

Files selected for processing: 1
  • Directory.Build.props

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.

@thomhurst

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T11:01:14.084482Z c011729 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Review: LGTM, with one minor note.

The change is small and correct. Defaulting SolutionDir and SolutionName in Directory.Build.props is conditioned on empty or *Undefined*. Solution builds and -p: overrides therefore still win, because global properties can't be overwritten by the props file. I grepped the repo and found no other consumer of SolutionDir or SolutionName in props, targets, csproj or workflows, so the repo-wide scope is low risk.

Notes:

  • $(MSBuildThisFileDirectory) ends with a trailing slash, which matches the SolutionDir convention. Good.
  • SolutionName=TUnit is applied to every project, including ones built through TUnit.Aspire or other .sln files. That is harmless today. If a future target keys off SolutionName, it will see TUnit for all of them. A short comment saying the value is for Verify only would help. Alternatively, scope the defaults to tests/*.Tests through a tests/Directory.Build.props, so production projects are untouched.
  • The validation in the description is good: the warning reproduced beforehand and was gone afterward, with no snapshot changes.

Approving from a design standpoint.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: c011729431

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps

greptile-apps Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Directory.Build.props now gives project-only builds an explicit Verify solution context.

  • Project-only builds now give Verify a default TUnit solution context.

Reviews (1) · Last reviewed commit: "Fix Verify solution context for project ..." · Reviewed by Greptile

@thomhurst

Copy link
Copy Markdown
Owner Author

Reviewed all review surfaces for c011729. Claude and Codex both approve, Greptile reports no findings, and there are no inline threads. Claude’s optional documentation note is already satisfied by the added comment: “Project-only builds need an explicit Verify context because this repo has multiple solutions.” Keeping the shared defaults preserves solution and command-line overrides as validated in the PR. Marking ready for CI.

@thomhurst
thomhurst marked this pull request as ready for review November 3, 2026 10:59
@thomhurst

thomhurst commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner Author

The earlier wait has cleared: promotion review run 41623865197 passed for unchanged head c011729. Re-read reviews, inline comments, review threads, and conversation comments: Claude and Codex have positive verdicts for this head, with no blocking findings or threads.

Disposition of Claude’s promotion notes: a repository search of tracked .props, .targets, and .csproj files found no existing SolutionDir/SolutionName consumers. Keep the shared defaults so all Verify snapshot projects receive consistent context; the guards preserve solution and command-line values, and the existing comment explains the Verify purpose. The narrower alternative is optional and unnecessary for this fix.

All CI is green, including all eight currently required checks. Assert-PrGreen.ps1 passed. Proceeding with the repository merge wrapper; this PR closes #7419.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Review

Small, focused change. Defaulting SolutionDir/SolutionName in Directory.Build.props with an empty/*Undefined* guard preserves values from solution builds and the command line. MSBuildThisFileDirectory ends in a slash, which matches the SolutionDir convention. The validation described in the PR is reasonable.

Things to consider (non-blocking):

  • Scope: SolutionDir is a widely consumed property (some NuGet/build-tool packages use it for relative paths). Defining it for every project, including project-only builds and the pipeline/template projects, could subtly change behavior. A grep for SolutionDir across the repo's .props/.targets/.csproj files would confirm nothing relies on it being undefined.
  • Narrower alternative: put these defaults in tests/TUnit.Core.SourceGenerator.Tests (or a Directory.Build.props there) so only Verify consumers get them. That avoids global side effects, at the cost of repeating the setting if other snapshot-test projects need it.
  • Evaluation order: Directory.Build.props is evaluated before the project body. Solution builds inject SolutionDir as a global property, so they win, but in a solution build SolutionName would be TUnit, TUnit.CI or whichever .slnx is used. Nothing here overrides that.

No blocking issues. Looks good to merge once CI is green.

@thomhurst
thomhurst merged commit b5561fb into main Nov 3, 2026
40 checks passed
@thomhurst
thomhurst deleted the issue-7419-verify-context branch November 3, 2026 13:58

This branch was previously deployed

1 inactive deployment
Pull Requests — c0117294 Deployed Nov 3, 2026 by thomhurst via modularpipeline (windows-latest) #20911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Generator snapshot test builds warn that Verify cannot choose among solution files

1 participant