Review simplify compatibility sources, deduplicate tests#15472
Conversation
…cate generated tests
| { | ||
| var dataRows = new List<RunnerInfo>(); | ||
|
|
||
| if (WithEveryVersionOfRunner) |
There was a problem hiding this comment.
There were two sources of the data, idea was that TestPlatformCompatibilitySource, has some more powerful api to generate exactly what is needed, but this makes it really hard to understand what it will do, and that source is for experimentation only anyway. So now we simply generate all combinations of the input data (with some special handling for in process - just because we don't have "unknown" for testhost version.
All in all this is not needed anymore, and was causing bugs for mstest source, which provided all test runner versions, and WithEveryVersionOfRunner = false, which had no effect. So tests were generated for every runner version, which does not make sense.
There was a problem hiding this comment.
Pull request overview
This PR simplifies and deduplicates the compatibility test data sources by splitting them into 4 focused levels: wrapper, runner, testhost, and adapter. It reduces test generation (e.g., MSTest source was generating 40 tests per runner/host instead of just adapter/host combinations). A new WrapperCompatibilityDataSource is introduced for VSTestConsoleWrapper-specific tests, while TestPlatformCompatibilityDataSource is renamed to CustomCompatibilityDataSource.
Changes:
- New
WrapperCompatibilityDataSourceadded; existing data sources simplified by removing configurable parameters in favour of fixed defaults. CompatibilityRowsBuilderrefactored: removed separateAddEveryVersionOf*/AddOlderConfigurationsmethods, replaced with a singleAddRowsmethod.- Multiple test methods updated to use
WrapperCompatibilityDataSourceinstead ofRunnerCompatibilityDataSource.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
Extension/WrapperCompatibilityDataSource.cs |
New data source for VSTestConsoleWrapper-specific compatibility tests |
Extension/RunnerCompatibilityDataSource.cs |
Simplified: removes configurable parameters, always sets WithInProcess = true and WithVSIXRunner = true |
Extension/TesthostCompatibilityDataSource.cs |
Simplified: removes configurable runner framework parameter, uses fixed runner defaults |
Extension/MSTestCompatibilityDataSource.cs |
Simplified: focuses on adapter vs. host combinations with fixed runner version |
Extension/TestPlatformCompatibilityDataSource.cs |
Renamed to CustomCompatibilityDataSource, simplified docstring |
Extension/CompatibilityRowsBuilder.cs |
Core refactor: removes separate per-dimension iteration methods, adds deduplication logic |
AcceptanceTestBase.cs |
Adds DEFAULT_HOST_NET and DEFAULT_RUNNER_NET constants |
ExecutionTests.cs |
Removes RunTestsFromMultipleMSTestAssemblies, renames RunMultipleTestAssemblies |
TranslationLayerTests/*.cs |
Updates data source attributes from RunnerCompatibilityDataSource to WrapperCompatibilityDataSource |
| { | ||
| // Older configurations where the runner, host and adapter version are the same. | ||
| // We already added the row where all are newest when adding combination with all runners. | ||
| foreach (var runnerVersion in _runnerVersions) |
There was a problem hiding this comment.
This whole method tries to be less clever. Whatever you specify, it will give you all combinations. Before we had different methods that tried to encode some of the thinking (e.g. when testing testhost, we don't need every version of runner), now instead this is done via the input data - you provide just 1 version of the runner to use, and so you will get 1 combination.
| //TODO: It looks like the first 3 tests would be useful to multiply by all 3 test frameworks, should we make the test even more generic, or duplicate them? | ||
| [TestMethod] | ||
| [TestCategory("Windows-Review")] | ||
| [MSTestCompatibilityDataSource(InProcess = true)] |
| [TestMethod] | ||
| [TestCategory("Windows-Review")] | ||
| [TestPlatformCompatibilityDataSource] | ||
| public void RunTestsFromMultipleMSTestAssemblies(RunnerInfo runnerInfo) |
There was a problem hiding this comment.
Same tests as above, just multiplied to 100+ tests which can test combinations of versions that are already released.
Co-authored-by: Copilot <[email protected]>
|
Passed windows tests, committing PR review comments from copilot. Ready to review. |
…ion/WrapperCompatibilityDataSource.cs Co-authored-by: Copilot <[email protected]>
…ion/CompatibilityRowsBuilder.cs Co-authored-by: Copilot <[email protected]>
…ion/CompatibilityRowsBuilder.cs Co-authored-by: Copilot <[email protected]>
|
failing on |
Description
Split the compatibility sources to 4 levels, wrapper, runner, testhost and adapter. Review how tests are generated, and reduce the number of tests generated. E.g. mstest source was generating 40 tests for each runner and host, instead of just combination of adapter versions and hosts, which is where adapter runs.
On the other hand RunnerCompatibilitySource generates by default tests for VSIX and inProcess.
Related issue
Related #15464
Related #15470
Fix #15454