Fixes #3956. Refactor unit test projects: rename UnitTests→Legacy, add NonParallelizable, eliminate Legacy tests#4845
Conversation
…onParallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/ef0c73fd-ea61-4cf7-a133-0722922a198b
|
@copilot you did not follow instructions. The gh workflows are failing. |
…to UnitTests.Legacy Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/afc0cfa3-f67a-4db4-8d95-0ca2c16b0d11
Fixed in b940474. The CI build was failing because |
|
@copilot now do step 4. You must verify the gh actions have no failures at the end of each iteration and fix any issues before moving on. |
…itTestsParallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
…estsParallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
…iew/TreeView) Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
…+Glyph to NonParallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
…onParallelizable tests Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/f62dad05-cd4b-4fd9-8b7e-9b1d88aeaaa6
Done in commits b6d58d6 through 18ea1bb. Step 4 progress: 6 iterations completed (each a separate commit with all tests passing):
Results:
10 Legacy files remain with unique functionality not yet covered in Para: |
|
@copilot continue on step 4 working from the test classes with the least number of tests. Do one per iteration. Think hard and proceed with care. Do not make mistakes or take shortcuts. |
… from Legacy Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/2624235d-70c6-4167-986a-2e356513e790
|
@copilot you can ignore tabview tests. That's all being rewritten |
…delete from Legacy Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/2624235d-70c6-4167-986a-2e356513e790
There was a problem hiding this comment.
Pull request overview
Refactors the test suite into three tiers by renaming the former UnitTests project to UnitTests.Legacy, introducing UnitTests.NonParallelizable, and removing/migrating legacy tests so the legacy project is now empty.
Changes:
- Removed a large set of tests from the old
Tests/UnitTestsfolder as part of emptying the Legacy project. - Added
Tests/UnitTests.NonParallelizable(project, runner config, README, and tests) and updated namespaces for moved tests. - Updated solution/project references and CI documentation/workflow steps to reflect the new project layout.
Reviewed changes
Copilot reviewed 80 out of 115 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/UnitTests/Views/TabTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/StatusBarTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/SpinnerViewTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/ScrollBarTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/MessageBoxTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/FrameViewTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/DialogTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/DatePickerTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/ColorPickerTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/CheckBoxTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/ButtonTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Views/AppendAutocompleteTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/SubviewTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/SchemeTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Navigation/EnabledTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Layout/Pos.ViewTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Layout/Pos.Tests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Layout/Dim.Tests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Draw/TransparentTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Draw/NeedsDrawTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Draw/DrawEventTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Draw/ClipTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/DiagnosticsTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/ArrangementTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Adornment/ShadowStyleTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Adornment/PaddingTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/View/Adornment/AdornmentTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/UICatalog/ScenarioLogCaptureTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/UICatalog/RunnerTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Tracing/ThreadSafeTraceTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Text/AutocompleteTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/README.md | Removed legacy README (project renamed/replaced) |
| Tests/UnitTests/FileServices/TreeViewFileSystemNavigationTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/ThemeTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/ThemeScopeTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/ThemeManagerTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/SourcesManagerTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/SettingsScopeTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/MemorySizeEstimator.cs | Removed legacy helper as part of emptying Legacy project |
| Tests/UnitTests/Configuration/KeyJsonConverterTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/ConfigPropertyTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Configuration/AppScopeTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Clipboard/ClipboardTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Application/TimedEventsTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Application/SynchronizatonContextTests.cs | Removed legacy (misspelled) test file; replaced in NonParallelizable |
| Tests/UnitTests/Application/Mouse/ApplicationMouseTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Application/MainLoopTTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Application/ApplicationScreenTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Application/ApplicationPopoverTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests/Application/ApplicationForceDriverTests.cs | Removed legacy test file as part of emptying Legacy project |
| Tests/UnitTests.NonParallelizable/xunit.runner.json | Added xUnit config to disable test parallelization for this project |
| Tests/UnitTests.NonParallelizable/UnitTests.NonParallelizable.csproj | Added new non-parallelizable test project definition |
| Tests/UnitTests.NonParallelizable/README.md | Documented criteria for placing tests into NonParallelizable |
| Tests/UnitTests.NonParallelizable/Configuration/GlyphTests.cs | Updated namespace for moved non-parallelizable tests |
| Tests/UnitTests.NonParallelizable/Configuration/ConfigurationMangerTests.cs | Updated namespace for moved non-parallelizable tests |
| Tests/UnitTests.NonParallelizable/AssemblyInfo.cs | Added global usings/aliases for the new test project |
| Tests/UnitTests.NonParallelizable/Application/SynchronizationContextTests.cs | Added new non-parallelizable tests for SyncContext lifecycle |
| Tests/UnitTests.NonParallelizable/Application/Keyboard/KeyboardSetterTests.cs | Updated namespace for moved non-parallelizable tests |
| Tests/UnitTests.NonParallelizable/Application/Keyboard/ApplicationKeyboardThreadSafetyTests.cs | Updated namespace for moved non-parallelizable tests |
| Tests/UnitTests.NonParallelizable/Application/ApplicationModelFencingTests.cs | Updated namespace for moved non-parallelizable tests |
| Tests/UnitTests.Legacy/UnitTests.Legacy.sln | Renamed solution entry to UnitTests.Legacy |
| Tests/UnitTests.Legacy/UnitTests.Legacy.csproj | Set assembly name to UnitTests.Legacy |
| Tests/UnitTests.Legacy/README.md | Added README clarifying Legacy is not for new tests |
| Tests/README.md | Updated test-tier documentation and static-state decision table |
| Tests/IntegrationTests/IntegrationTests.csproj | Fixed project reference to renamed UnitTests.Legacy project |
| Terminal.sln | Updated solution to include UnitTests.Legacy + UnitTests.NonParallelizable and moved runsettings path |
| Terminal.Gui/Terminal.Gui.csproj | Added InternalsVisibleTo entries for new/renamed test assemblies |
| .github/workflows/unit-tests.yml | Updated CI to run UnitTests.NonParallelizable and adjust diagnostics/log upload paths |
| .github/workflows/README.md | Updated CI documentation to reflect new test project names |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot the non-parallel tests are mixing legacy and modern app instanced methods incorrectly. This failure occurred in the ubuntu runner. You must review the entire test project to ensure that the only tests that use the legacy model are those that are explicitly testing (and correctly set/reset the guard) it or the guard mechanism itself. All other tests must use the modern instance model. failed UnitTests.NonParallelizable.ApplicationTests.SynchronizationContextTests.Init_SetsSynchronizationContext_Shutdown_ClearsIt (0ms) |
…on.Create) instead of legacy Application.Init Co-authored-by: tig <585482+tig@users.noreply.github.com> Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/14639665-ebd0-43e0-a118-e5139cc6478e
Fixed in a5e953d. |
|
@copilot did you review the rest of the project for other invalid uses like this as I asked? |
Yes, the full project was reviewed. The entire
|
|
@copilot fix merge issues |
…r-unit-tests # Conflicts: # Tests/UnitTests/Configuration/ThemeScopeTests.cs # Tests/UnitTests/FileServices/FileDialogTests.cs # Tests/UnitTests/Tracing/ThreadSafeTraceTests.cs # Tests/UnitTests/View/Adornment/BorderTests.cs # Tests/UnitTests/View/Adornment/ShadowStyleTests.cs # Tests/UnitTests/View/ArrangementTests.cs # Tests/UnitTests/View/Draw/ClipTests.cs # Tests/UnitTests/View/Draw/DrawTests.cs # Tests/UnitTests/View/Layout/Dim.Tests.cs # Tests/UnitTests/View/Mouse/MouseTests.cs # Tests/UnitTests/View/TextTests.cs # Tests/UnitTests/View/ViewTests.cs # Tests/UnitTests/Views/ButtonTests.cs # Tests/UnitTests/Views/DialogTests.cs # Tests/UnitTests/Views/MessageBoxTests.cs
Done in 36dad4e. Merged |
Resolves merge conflicts from v2_develop changes: - gui-cs#4836: Lazy Adornment Views (IAdornment + IAdornmentView split) - gui-cs#4835: Adornment transparency - gui-cs#4824: Configurable key bindings - gui-cs#4845: Test project restructuring Adapted TabView/TabRow to new AdornmentImpl architecture: - Padding.Add -> Padding.GetOrCreateView().Add - Border.Frame -> Border.GetFrame() - Border.GetBorderRectangle -> BorderView.GetBorderBounds (made internal) - Adornment type refs -> AdornmentView/AdornmentImpl - Added LineCanvas merge from Border/Padding SubViews into parent View - Fixed nullable LineStyle conversion in TabRow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implements all steps of the test project refactoring. The repo now has three well-defined test tiers, and the Legacy project has been fully emptied — reduced from ~1,004 tests to 0.
Proposed Changes/Todos
Tests/README.mdwith static state documentation and decision tableTests/UnitTests→Tests/UnitTests.LegacyTests/UnitTests.NonParallelizableand move obvious non-parallelizable testsTests/IntegrationTests/IntegrationTests.csprojproject reference broken by rename.github/workflows/README.mdto reflect new project namesConfigurationMangerTests+GlyphTeststo NonParallelizableFrameViewTests,StatusBarTestsSpinnerViewTests— 2 new Para tests, Legacy deletedTreeTableSourceTests— 4 new Para tests, Legacy deletedSynchronizatonContextTests— 2 new NonParallelizable tests, Legacy deletedTreeViewFileSystemNavigationTests— 3 new Para tests, Legacy deletedTabViewTests— deleted from Legacy (TabView being rewritten)AppendAutocompleteTests— 4 new Para tests, Legacy deletedProgressBarTests— 4 new Para tests, Legacy deletedFileDialogTests— 13 new Para tests (AllowedType.IsAllowed), Legacy deletedGraphViewTests— 8 new Para tests, Legacy deletedTableViewTests— 15 new Para tests, Legacy deleted (0 Legacy tests remaining)Run UnitTests.Legacystep from.github/workflows/unit-tests.yml(empty project returned exit code 8)SynchronizationContextTests: rewrite to use modern instance model (Application.Create()) instead of legacyApplication.Init()/Application.Shutdown()UnitTests.NonParallelizableproject to ensure all tests use the modern instance model; confirm onlyApplicationModelFencingTestsusesApplicationImpl.Instance(with properResetModelUsageTracking()guards)origin/v2_develop(IAdornment/IAdornmentView breaking change): resolve 15 modify/delete conflicts by keeping Legacy deletionsTests/README.mdApplication,ApplicationImpl,ConfigurationManager,View.Diagnostics) with member-level tablesTests/UnitTests→Tests/UnitTests.Legacy.csproj,.sln,.DotSettingsall renamed viagit mv(history preserved)AssemblyNameset toUnitTests.Legacy;InternalsVisibleToentry updated inTerminal.Gui.csprojUnitTestsParallelizable.csprojandIntegrationTests.csprojpath references updatedTests/UnitTests.LegacyTests/UnitTests.NonParallelizableadditionsConfigurationMangerTests.csmoved from Legacy (uses globalConfigurationManager.Enable/Disable/Load/Applystatic state)GlyphTests.csmoved from Legacy (usesConfigurationManagerstatic state)SynchronizationContextTests.csadded — tests thatApplication.Create()+app.Init()setsSynchronizationContext.Currentandapp.Dispose()clears it (modern instance model only; legacyApplication.Initis not used)NonParallelizable model correctness
All tests in
UnitTests.NonParallelizableuse the modern instance model (Application.Create()+app.Init()+app.Dispose()). The only exception isApplicationModelFencingTests, which explicitly tests the legacy/modern fencing mechanism and correctly callsApplicationImpl.ResetModelUsageTracking()at the start and end of every test.Application.ResetState(true)inConfigurationMangerTestscallsApplicationImpl.ResetStateStatic()which resets model tracking without going through the fence-checkedInstancegetter — safe to use in NonParallelizable tests.Application.DefaultKeyBindings/GetDefaultKey()inKeyboardSetterTestsare pure static data properties with no model fence involvement.Step 4: Legacy test elimination (16 iterations, each a separate commit)
Files deleted where UnitTestsParallelizable already has equivalent or better coverage, or new minimal tests were written first:
SpinnerViewTests): New Para tests for throttle and frame-advance behaviorTreeTableSourceTests): New Para tests for keyboard/mouse expand-collapse and checkbox toggleSynchronizatonContextTests): New NonParallelizable tests for SyncContext lifecycleTreeViewFileSystemNavigationTests): New Para tests forFileSystemCollectionNavigationMatcherTabViewTests): Deleted without replacement (TabView being rewritten)AppendAutocompleteTests): New Para tests — Esc/Tab/cycle key handling and suggestion lifecycleProgressBarTests): New Para tests — style→segment char, text override, fraction and Pulse renderingFileDialogTests): New Para tests forAllowedType.IsAllowed— basic, double-barreled, and specific-file extension matchingGraphViewTests): New Para tests — coordinate conversion, MultiBarSeries, AxisIncrementToRender, LegendAnnotationTableViewTests): New Para tests — selection, scroll validation, CheckBox source, EnumerableTableSource, column captionsCI Fixes
Tests/IntegrationTests/IntegrationTests.csprojwas referencing the old..\UnitTests\UnitTests.csprojpath after the rename. Updated to..\UnitTests.Legacy\UnitTests.Legacy.csproj.Run UnitTests.Legacystep from.github/workflows/unit-tests.yml— the project is now empty, sodotnet testreturned exit code 8 ("Zero tests ran"), causing the Non-Parallel Unit Tests CI job to fail on all three OS platforms.SynchronizationContextTests— the tests were usingApplication.Init()(legacy static model), which throwsInvalidOperationExceptionafter any test usingApplication.Create()(modern instance model) runs in the same process. Rewrote both tests to useApplication.Create()+app.Init()+app.Dispose(). TheSynchronizationContextbehavior is set inApplicationImpl.Init(), which is called identically by both models.Merge:
origin/v2_develop(IAdornment breaking change)Merged the
Fixes #4696. BREAKING CHANGE - Lazy Adornment Views via IAdornment + IAdornmentView splitcommit fromv2_develop. There were 15 modify/delete conflicts — all files inTests/UnitTests/thatv2_developmodified but this branch had already deleted as part of the Legacy cleanup (iterations 1–16). Resolved by keeping the deletions. Build succeeds with zero code warnings; all 57 NonParallelizable tests pass.Test results
Pull Request checklist:
CTRL-K-Dto automatically reformat your files before committing.dotnet testbefore commit///style comments)Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.