Avoid MSB4011 duplicate-import warnings when layering MSTest.Sdk on another base SDK#9565
Open
Evangelink wants to merge 2 commits into
Open
Avoid MSB4011 duplicate-import warnings when layering MSTest.Sdk on another base SDK#9565Evangelink wants to merge 2 commits into
Evangelink wants to merge 2 commits into
Conversation
…nother base SDK (#9562) Guard the Microsoft.NET.Sdk import in MSTest.Sdk's Sdk.props/Sdk.targets behind a new _MSTestSdkImportsMicrosoftNETSdk property. MSTest.Sdk now only imports the base .NET SDK when no other SDK has already done so (detected via UsingMicrosoftNETSdk, which Microsoft.NET.Sdk sets very early). This lets MSTest.Sdk be layered on top of a different base SDK such as Microsoft.NET.Sdk.Web via manual SDK imports without emitting MSB4011 warnings. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves MSTest.Sdk’s behavior when it’s layered on top of another SDK (e.g., Microsoft.NET.Sdk.Web) via manual Sdk.props/Sdk.targets imports by preventing duplicate imports of Microsoft.NET.Sdk that would otherwise produce MSB4011 warnings.
Changes:
- Add
_MSTestSdkImportsMicrosoftNETSdkinSdk.propsto detect whether an outer SDK already importedMicrosoft.NET.Sdk, and conditionally skip re-importing it. - Use the same
_MSTestSdkImportsMicrosoftNETSdkdecision inSdk.targetsto keep props/targets imports in sync and avoid duplicate-import warnings. - Add an acceptance test that builds and runs a
Microsoft.NET.Sdk.Web+MSTest.Sdkmanually-layered project and asserts noMSB4011appears in build output; document the layering pattern in the SDK README.
Show a summary per file
| File | Description |
|---|---|
| test/IntegrationTests/MSTest.Acceptance.IntegrationTests/SdkTests.cs | Adds an acceptance test covering the mixed/manual SDK import scenario and asserting MSB4011 is absent. |
| src/Package/MSTest.Sdk/Sdk/Sdk.targets | Conditionally imports Microsoft.NET.Sdk targets only when MSTest.Sdk performed the base SDK import. |
| src/Package/MSTest.Sdk/Sdk/Sdk.props.template | Computes _MSTestSdkImportsMicrosoftNETSdk and conditionally imports Microsoft.NET.Sdk props to prevent duplicate-import warnings. |
| src/Package/MSTest.Sdk/README.md | Documents the recommended manual layering pattern (base SDK first) and explains the guard behavior. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Low
…l SDK imports The versioned SDK form (MSTest.Sdk/version) only resolves on the outer <Project Sdk> attribute, not on <Import Sdk=...>, which caused MSB4236 'SDK could not be found' in CI. Pin the version through global.json's msbuild-sdks instead, matching the canonical way to pin MSBuild project SDK versions for manual/mixed imports. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
🧪 Test quality grade — PR #9565
This advisory comment was generated automatically. Grades are heuristic
|
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
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.
Fixes #9562.
Problem
MSTest.Sdkimplicitly importsMicrosoft.NET.Sdkas its base SDK in bothSdk.propsandSdk.targets. WhenMSTest.Sdkis layered on top of a different base SDK (for exampleMicrosoft.NET.Sdk.Webfor an ASP.NET Core integration test project) using manual SDK imports, the baseMicrosoft.NET.Sdkgets imported twice and the build emitsMSB4011duplicate-import warnings:Fix
Guard the
Microsoft.NET.Sdkimport behind a new_MSTestSdkImportsMicrosoftNETSdkproperty.Microsoft.NET.SdksetsUsingMicrosoftNETSdk=truevery early (before anything else), soMSTest.Sdkcan detect whether an outer SDK has already imported the base SDK and skip re-importing it. The decision is recorded inSdk.propsand reused inSdk.targetsso both stay in sync — when layered, the outer SDK owns the base import.The common
<Project Sdk="MSTest.Sdk">usage is unchanged:UsingMicrosoftNETSdkis unset at that point, soMSTest.Sdkstill imports the base SDK exactly as before.Changes
src/Package/MSTest.Sdk/Sdk/Sdk.props.template— compute_MSTestSdkImportsMicrosoftNETSdkand gate the base props import on it.src/Package/MSTest.Sdk/Sdk/Sdk.targets— gate the base targets import on the same property.src/Package/MSTest.Sdk/README.md— document the first-class layering pattern (import the base SDK first).test/IntegrationTests/MSTest.Acceptance.IntegrationTests/SdkTests.cs— new acceptance testMSTestSdk_LayeredOnTopOfWebSdk_BuildsWithoutDuplicateImportWarningsthat builds aMicrosoft.NET.Sdk.Web+MSTest.Sdkproject, asserts it runs, and asserts noMSB4011in the output.Verification
Reproduced the scenario with the real base SDKs plus a control:
... cannot be imported again(MSB4011)