Split integration tests to single tfm and multi tfm project#15484
Split integration tests to single tfm and multi tfm project#15484nohwnd merged 21 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restructures integration testing by separating single-TFM acceptance/integration tests from multi-TFM “in-proc” library integration tests, and centralizes shared integration-test build/setup logic.
Changes:
- Moved/centralized integration-test asset build + dotnet patching into
IntegrationTestBuild(TestUtilities) and wired test assembly initialization to it. - Split integration test projects: Acceptance.IntegrationTests becomes single-TFM, and a new multi-targeted Library.IntegrationTests project is introduced.
- Added new build script switches/filters to support smoke/compatibility/performance test slices.
Reviewed changes
Copilot reviewed 58 out of 87 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.TestPlatform.TestUtilities/Microsoft.TestPlatform.TestUtilities.csproj | Adds package references needed by shared test build/utilities code. |
| test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs | New centralized build/setup for integration tests (assets restore/build, compatibility matrix, dotnet patching). |
| test/Microsoft.TestPlatform.TestUtilities/CompatibilityDataSourceAttribute.cs | Introduces a shared “marker” base attribute for compatibility data sources. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/README.MD | Documents the purpose of the new in-proc multi-TFM integration test project. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/Properties/AssemblyInfo.cs | Enables MSTest parallelization for the new project. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/Microsoft.TestPlatform.Library.IntegrationTests.csproj | New multi-targeted integration test project definition. |
| test/Microsoft.TestPlatform.Library.IntegrationTests/BannedSymbols.txt | Adds banned API list for the new integration test project. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TranslationLayerTests/RunTests.cs | Adds Smoke categorization to a specific test. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TestPlatformNugetPackageTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TestCaseFilterTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/TelemetryTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/SkipIOutOfProcessTestOnNetFrameworkConditionAttribute.cs | Removes the NetFx duplication-skip attribute implementation. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/SelfContainedAppTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/RunsettingsTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ResultsDirectoryTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/RecursiveResourcesLookupTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/README.MD | Documents the purpose of the (now single-TFM) acceptance integration tests project. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ProcessesInteractionTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/PostProcessingTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/PortableNugetPackageTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/PlatformTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MultitargetingTestHostTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Microsoft.TestPlatform.Acceptance.IntegrationTests.csproj | Changes Acceptance.IntegrationTests to single-TFM and removes some package/framework-specific refs. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/LoggerTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ListExtensionsTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/FrameworkTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/FilePatternParserTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/WrapperCompatibilityDataSource.cs | Switches compatibility data sources to derive from the new marker base attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/TesthostCompatibilityDataSource.cs | Switches compatibility data sources to derive from the new marker base attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/TestDataSourceAttribute.cs | Changes AddData to accept TestDataRow<T> (to carry categories/metadata). |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/RunnnerInfo.cs | Updates to use StringUtils.IsNullOrEmpty. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/RunnerCompatibilityDataSource.cs | Switches compatibility data sources to derive from the new marker base attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/MSTestCompatibilityDataSource.cs | Switches compatibility data sources to derive from the new marker base attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/CustomCompatibilityDataSource.cs | Switches compatibility data sources to derive from the new marker base attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension/CompatibilityRowsBuilder.cs | Returns TestDataRow<RunnerInfo> and stamps Compatibility category. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ExecutionThreadApartmentStateTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/ExecutionTests.cs | Removes NetFx duplication-skip condition attribute; adds a new smoke-categorized test. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/EventLogCollectorTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetTestTests.cs | Removes NetFx duplication-skip condition attribute; adds Smoke categorization. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetTestMSBuildOutputTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetHostArchitectureVerifierTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DotnetArchitectureSwitchTests.Windows.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DiscoveryTests.cs | Removes NetFx duplication-skip condition attribute; adds Smoke categorization. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DisableAppdomainTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DifferentTestFrameworkSimpleTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DebugAssertTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectorTests.Coverlet.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectorAttachmentsProcessorsFactoryTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DataCollectionTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/CodeCoverageTests.cs | Removes NetFx duplication-skip condition attribute. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/CodeCoverageAcceptanceTestBase.cs | Uses StringUtils.IsNullOrEmpty instead of string.IsNullOrEmpty. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/Build.cs | Simplifies acceptance test assembly initialization to call the shared build helper. |
| test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs | Removes NetFx duplication-skip condition attribute. |
| src/vstest.console/Friends.cs | Grants internals access to the new Library.IntegrationTests assembly. |
| src/Microsoft.TestPlatform.Common/Friends.cs | Grants internals access to the new Library.IntegrationTests assembly. |
| eng/build.ps1 | Adds new CLI switches and constructs MSTest filter/test-parameter arguments. |
| TestPlatform.sln | Adds new test project and updates solution metadata. |
| Build.cs | Adds a solution-level Build entrypoint that calls the centralized integration-test build helper. |
Comments suppressed due to low confidence (3)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
SkipIntegrationTestBuildis logged but does not actually skip the build (noreturn/ early-exit). This makes the switch ineffective and can still trigger expensive restores/builds. Add an early return after logging (or wrap the build logic in anif (!skipBuild)block).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1- Parameter name is misspelled (
BuildCompatiblity). The build script setsBuildCompatibility, so this will always read asfalseand compatibility assets will never build. Rename the parameter key toBuildCompatibility(and update any related log messages using the misspelling).
test/Microsoft.TestPlatform.TestUtilities/CompatibilityDataSourceAttribute.cs:1 CompatibilityDataSourceAttributeis added under the TestUtilities project but inherits fromTestDataSourceAttribute<RunnerInfo>, which (per this PR) is defined undertest/Microsoft.TestPlatform.Acceptance.IntegrationTests/Extension. This creates a tight coupling between a shared utilities project and acceptance-test-specific extension types. Consider movingCompatibilityDataSourceAttributenext toTestDataSourceAttribute(same project), or moving the sharedTestDataSourceAttribute/RunnerInfoabstractions into TestUtilities so dependencies flow one way.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 93 out of 94 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (6)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
SkipIntegrationTestBuildis logged but the method does not return, so the build still proceeds. If this flag is intended to prevent the expensive asset build (as suggested by the build script), add an early return whenskipBuildis true.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1- The test-parameter name has a typo (
BuildCompatiblity). Ineng/build.ps1the parameter is set asBuildCompatibility, so this will never be read as true and compatibility assets won’t build. Rename the parameter key here to match the script (and keep spelling consistent everywhere).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - PATH is being prepended using a hard-coded
;separator, which will break PATH on non-Windows where the separator is:. UsePath.PathSeparator(or OS-conditional logic) to join PATH entries.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 generatedSlnis set toCompatibilityTestAssets.slnx, butdotnet new slntypically creates a.slnfile unless explicitly configured otherwise. If the file extension doesn’t match what is created, subsequentdotnet sln/restore/buildcalls will fail. Consider aligninggeneratedSlnwith the actual output (or pass an explicit argument/format that produces.slnx).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1generatedSlnis set toCompatibilityTestAssets.slnx, butdotnet new slntypically creates a.slnfile unless explicitly configured otherwise. If the file extension doesn’t match what is created, subsequentdotnet sln/restore/buildcalls will fail. Consider aligninggeneratedSlnwith the actual output (or pass an explicit argument/format that produces.slnx).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1- Correct the comment typo: “I any” → “If any”.
…stest into split-integration-tests
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 92 out of 93 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (7)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
- The test parameter name is misspelled (
BuildCompatiblity) and doesn’t match the name being set from the build script (BuildCompatibility). This will cause compatibility asset build to never run even when-compatibilityTestis requested; rename the parameter key (and the log message) toBuildCompatibilityconsistently.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - The test parameter name is misspelled (
BuildCompatiblity) and doesn’t match the name being set from the build script (BuildCompatibility). This will cause compatibility asset build to never run even when-compatibilityTestis requested; rename the parameter key (and the log message) toBuildCompatibilityconsistently.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - When
SkipIntegrationTestBuildis true, the code logs but continues into the mutex/build path anyway. If the intent is to skip the expensive one-time build in split jobs, this should return early (or otherwise bypass the build steps) to make the flag effective.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, the mutex is not acquired, butReleaseMutex()is still called infinally, which will throw and can mask the originalTimeoutException. Track whether the mutex is owned before releasing (e.g., only release ifcreatedNewis true orgotOneis true).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, the mutex is not acquired, butReleaseMutex()is still called infinally, which will throw and can mask the originalTimeoutException. Track whether the mutex is owned before releasing (e.g., only release ifcreatedNewis true orgotOneis true).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, the mutex is not acquired, butReleaseMutex()is still called infinally, which will throw and can mask the originalTimeoutException. Track whether the mutex is owned before releasing (e.g., only release ifcreatedNewis true orgotOneis true).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - This prepends to PATH using
;, which is Windows-specific. UsePath.PathSeparatorto keep PATH well-formed on Unix-like agents (even if you usually invokedotnetvia absolute path, other invoked tools may rely on PATH).
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 92 out of 93 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
- When
SkipIntegrationTestBuildis true the method currently only logs but continues into the build path. To actually skip the shared build (as intended for split CI jobs), return immediately after logging or wrap the remaining logic in anif (!skipBuild)block.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - The test parameter name is misspelled (
BuildCompatiblity) and does not match the build script which setsBuildCompatibility. As a result, compatibility builds will never be enabled via--test-parameter. Rename the parameter key consistently (and update the associated log message strings).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, the code throws but still callsReleaseMutex()infinallywithout owning the mutex, which can raiseApplicationExceptionand mask the original timeout. Track whether the mutex was successfully acquired (e.g.,acquired = createdNew || gotOne) and only callReleaseMutex()when acquired.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, the code throws but still callsReleaseMutex()infinallywithout owning the mutex, which can raiseApplicationExceptionand mask the original timeout. Track whether the mutex was successfully acquired (e.g.,acquired = createdNew || gotOne) and only callReleaseMutex()when acquired.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, the code throws but still callsReleaseMutex()infinallywithout owning the mutex, which can raiseApplicationExceptionand mask the original timeout. Track whether the mutex was successfully acquired (e.g.,acquired = createdNew || gotOne) and only callReleaseMutex()when acquired.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 generatedSlnis hardcoded to.slnx, butdotnet new slntypically produces a.slnunless explicitly configured. This can cause subsequentdotnet sln/restore/buildcalls to fail because the solution path doesn't exist. Consider using.slnor passing an explicit format option (and keep the extension consistent everywhere).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1generatedSlnis hardcoded to.slnx, butdotnet new slntypically produces a.slnunless explicitly configured. This can cause subsequentdotnet sln/restore/buildcalls to fail because the solution path doesn't exist. Consider using.slnor passing an explicit format option (and keep the extension consistent everywhere).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1- Copying files into
compatibilityProjectDirwill fail for nested paths (e.g.,Properties\\AssemblyInfo.cs) because the destination subdirectories are not created beforeFile.Copy. CreatePath.GetDirectoryName(fullPath)(when non-null) before copying, or replace this loop with a directory-copy routine that preserves structure.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 95 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
- The
SkipIntegrationTestBuildflag logs that the build is being skipped, but the method continues and will still take the mutex and run the build steps. Return early after the common setup whenskipBuildis true (or gate the build section on!skipBuild) so the flag actually has effect.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - The test parameter name is misspelled (
BuildCompatiblity). Ineng/build.ps1the parameter is set asBuildCompatibility, so compatibility builds will never be enabled. Rename this key toBuildCompatibility(and keep spelling consistent across all producers/consumers).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - If
WaitOne(...)times out, aTimeoutExceptionis thrown andReleaseMutex()is still called infinally, but the mutex won't be owned by this thread—ReleaseMutex()will throw and can mask the real timeout. Track whether the mutex was successfully acquired and only release when owned (e.g.,acquired = createdNew || gotOne).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - Copying files into
fullPathcan throwDirectoryNotFoundExceptionfor nested paths because parent directories aren’t created. CreatePath.GetDirectoryName(fullPath)first (when non-null) andDirectory.CreateDirectory(...)before callingFile.Copy.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs:1 - Correct the grammar in the assert message: “You need to changed” → “You need to change”.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs
Show resolved
Hide resolved
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/DifferentTestFrameworkSimpleTests.cs
Show resolved
Hide resolved
...atform.Acceptance.IntegrationTests/Microsoft.TestPlatform.Acceptance.IntegrationTests.csproj
Show resolved
Hide resolved
test/Microsoft.TestPlatform.Library.IntegrationTests/Properties/AssemblyInfo.cs
Show resolved
Hide resolved
test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/CodeCoverageTests.cs
Show resolved
Hide resolved
test/Microsoft.TestPlatform.Library.IntegrationTests/TranslationLayerTests/CodeCoverageTests.cs
Show resolved
Hide resolved
...TestPlatform.Library.IntegrationTests/Microsoft.TestPlatform.Library.IntegrationTests.csproj
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 94 out of 95 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
- The
SkipIntegrationTestBuildflag logs that it will skip, but the method continues and still runs the build. Also, the compatibility flag key is misspelled (BuildCompatiblity), which won’t match theBuildCompatibilityparameter being set ineng/build.ps1, so compatibility builds will never execute. MakeskipBuildreturn early, and standardize the test-parameter key spelling (e.g.,BuildCompatibility) across both the build script and this method.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 ReleaseMutex()is called unconditionally. IfWaitOne()times out (or throws) in thecreatedNew == falsepath, the mutex was never acquired by this process andReleaseMutex()will throw, potentially masking the real failure. Track whether the mutex was actually acquired (or only release whencreatedNew == true/gotOne == true) to avoid releasing when not owned.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs:1- Fix grammar in the assertion message: “You need to changed” should be “You need to change” (or “You may have changed…” if that’s the intent).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
SkipIntegrationTestBuildis read and logged but does not actually prevent the build/unzip/patch steps from running. If the intent is to skip rebuilding test assets (e.g., when other jobs already built them), add an early exit/fast-path (while still ensuring any required environment setup, like DOTNET_ROOT/PATH, is applied).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1- If
WaitOne(...)times out, aTimeoutExceptionis thrown andfinallywill still executeReleaseMutex(). At that point the current thread does not own the mutex, which will causeApplicationExceptionand mask the real timeout. Track mutex ownership (e.g., abool ownsMutex) and only callReleaseMutex()when the mutex was successfully acquired.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestEnvironment.cs:1 - This updated assertion message hard-codes a specific likely cause (compatibility assets not being built automatically). Since
GetTestAssetappears to be used for non-compatibility assets too, this can be misleading and slow down debugging. Consider reverting to a more general message (or include both: the missing path plus a hint about compatibility-build toggles only whenassetName/targetFrameworkindicates a compatibility asset).
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1 - Correct the spelling of 'BuildCompatiblity' to 'BuildCompatibility' in the log message.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 95 out of 96 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1
SkipIntegrationTestBuildcurrently does not skip anything (it only logs). If a caller sets this,BuildTestAssetsForIntegrationTestswill still acquire the mutex and run the build. Add an early return (or a branch that still waits for an ongoing build but does not start one) so the parameter works as intended.
test/Microsoft.TestPlatform.TestUtilities/IntegrationTestBuild.cs:1generatedSlnis hard-coded to.slnx, but later the code callsdotnet new sln --name CompatibilityTestAssets ...without specifying a format. Ifdotnet new slnproduces a.sln(common default), subsequentrestore/build/sln addcalls will point at a non-existent.slnxfile and fail. Make the filename/extension match what’s actually generated (or pass an explicit format option and keep the extension consistent).
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs
Show resolved
Hide resolved
Evangelink
left a comment
There was a problem hiding this comment.
Nice restructuring — splitting to single-tfm and multi-tfm test projects makes the intent much clearer, and the new build.ps1 filter/category machinery is a good step toward faster PR pipelines.
A few things I noticed:
Bugs (please fix):
SkipIntegrationTestBuildis checked inIntegrationTestBuild.BuildTestAssetsForIntegrationTestsbut there's no early return — the build always runs regardless of the flag.- Several
Assertmessages inEndSessionShouldEnsureVstestConsoleProcessDies(both inRunTests.csandCodeCoverageTests.cs) have swapped/misleading failure messages that will be confusing if they ever fire. DiscoverTests.cs: The release/debug line-number conditional now has both branches asserting22, making it dead code.
Build script concern:
4. In build.ps1, $PSBoundParameters['properties'] += ... may produce a [string] instead of [String[]] when no properties were passed originally.
Nits:
5. Double InvokeVsTestForExecution in RunMultipleMSTestAssembliesOnVstestConsoleAndTesthostCombinations3 without explanation.
6. Truncated comment in AppDomainTests.cs, typo in TestCaseFilterTests.cs.
Description
Add categories of tests, so we can run the most basic integration tests. Avoid running compatibility tests in PR pipeline (currently they run in the main pipeline only, but should change to run in separate nightly task, to avoid failing in the internal pipeline when we need to iterate fast.)
Split tests into 2 projects to make it explicit where we run tests in child process and where we need to test the code that is loaded into the test project.
Related issue
Related #15470