Skip to content

Commit ed64f57

Browse files
Copilottig
Copilot
and
tig
authored
Fixes #4282 - Migrate tests from UnitTests to UnitTests.Parallelizable (67 tests migrated + comprehensive documentation) (#4293)
* Initial plan * Add 31 parallelizable LineCanvas unit tests Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add 4 parallelizable Ruler unit tests Co-authored-by: tig <585482+tig@users.noreply.github.com> * Complete Category A migration - 35 tests migrated Co-authored-by: tig <585482+tig@users.noreply.github.com> * Remove duplicate tests from UnitTests after migration to Parallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> * Migrate 6 pure unit tests from ColorPicker and DatePicker to Parallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> * Fix duplicate test names between UnitTests and Parallelizable Co-authored-by: tig <585482+tig@users.noreply.github.com> * Migrate 11 Label tests to Parallelizable (52 tests total) Co-authored-by: tig <585482+tig@users.noreply.github.com> * Fix documentation: SetupFakeDriver tests CAN be parallelized Co-authored-by: tig <585482+tig@users.noreply.github.com> * Add comprehensive parallelization rules to documentation Co-authored-by: tig <585482+tig@users.noreply.github.com> * Update README and start migrating Button/CheckBox/RadioGroup tests Co-authored-by: tig <585482+tig@users.noreply.github.com> * Remove duplicate Button tests from UnitTests Co-authored-by: tig <585482+tig@users.noreply.github.com> * Migrate 5 CheckBox unit tests to Parallelizable (27 with Theory expansion) Co-authored-by: tig <585482+tig@users.noreply.github.com> * Migrate 4 more CheckBox tests to Parallelizable (67 tests total, 9,478 passing) Co-authored-by: tig <585482+tig@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tig <585482+tig@users.noreply.github.com> Co-authored-by: Tig <tig@users.noreply.github.com>
1 parent b6fff43 commit ed64f57

File tree

16 files changed

+1255
-834
lines changed

16 files changed

+1255
-834
lines changed

‎Tests/CATEGORY_A_MIGRATION_SUMMARY.md‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
# Category A Migration Summary
2+
3+
## Overview
4+
5+
This document summarizes the Category A test migration effort to move parallelizable unit tests from `UnitTests` to `UnitTests.Parallelizable`.
6+
7+
## Tests Migrated: 35
8+
9+
### Drawing/LineCanvasTests.cs: 31 tests
10+
**Migrated pure unit tests that don't require Application.Driver:**
11+
- ToString_Empty (1 test)
12+
- Clear_Removes_All_Lines (1 test)
13+
- Lines_Property_Returns_ReadOnly_Collection (1 test)
14+
- AddLine_Adds_Line_To_Collection (1 test)
15+
- Constructor_With_Lines_Creates_Canvas_With_Lines (1 test)
16+
- Viewport_H_And_V_Lines_Both_Positive (7 test cases)
17+
- Viewport_H_Line (7 test cases)
18+
- Viewport_Specific (1 test)
19+
- Bounds_Empty_Canvas_Returns_Empty_Rectangle (1 test)
20+
- Bounds_Single_Point_Zero_Length (1 test)
21+
- Bounds_Horizontal_Line (1 test)
22+
- Bounds_Vertical_Line (1 test)
23+
- Bounds_Multiple_Lines_Returns_Union (1 test)
24+
- Bounds_Negative_Length_Line (1 test)
25+
- Bounds_Complex_Box (1 test)
26+
- ClearExclusions_Clears_Exclusion_Region (1 test)
27+
- Exclude_Removes_Points_From_Map (1 test)
28+
- Fill_Property_Can_Be_Set (1 test)
29+
- Fill_Property_Defaults_To_Null (1 test)
30+
31+
**Tests that remain in UnitTests as integration tests:**
32+
- All tests using GetCanvas() and View.Draw() (16 tests)
33+
- Tests that verify rendered output (ToString with specific glyphs) - these require Application.Driver for glyph resolution
34+
35+
### Drawing/RulerTests.cs: 4 tests
36+
**Migrated pure unit tests:**
37+
- Constructor_Defaults
38+
- Attribute_Set
39+
- Length_Set
40+
- Orientation_Set
41+
42+
**Tests that remain in UnitTests as integration tests:**
43+
- Draw_Default (requires Application.Init with [AutoInitShutdown])
44+
- Draw_Horizontal (uses [SetupFakeDriver] - could potentially be migrated)
45+
- Draw_Vertical (uses [SetupFakeDriver] - could potentially be migrated)
46+
47+
## Key Findings
48+
49+
### 1. LineCanvas and Rendering Dependencies
50+
**Issue:** LineCanvas.ToString() internally calls GetMap() which calls GetRuneForIntersects(Application.Driver). The glyph resolution depends on Application.Driver for:
51+
- Configuration-dependent glyphs (Glyphs class)
52+
- Line intersection character selection
53+
- Style-specific characters (Single, Double, Heavy, etc.)
54+
55+
**Solution:** Tests using [SetupFakeDriver] CAN be parallelized as long as they don't use Application statics. This includes rendering tests that verify visual output with DriverAssert.
56+
57+
### 2. Test Categories
58+
Tests fall into three categories:
59+
60+
**a) Pure Unit Tests (CAN be parallelized):**
61+
- Tests of properties (Bounds, Lines, Length, Orientation, Attribute, Fill)
62+
- Tests of basic operations (AddLine, Clear, Exclude, ClearExclusions)
63+
- Tests that don't require Application static context
64+
65+
**b) Rendering Tests with [SetupFakeDriver] (CAN be parallelized):**
66+
- Tests using [SetupFakeDriver] without Application statics
67+
- Tests using View.Draw() and LayoutAndDraw() without Application statics
68+
- Tests that verify visual output with DriverAssert (when using [SetupFakeDriver])
69+
- Tests using GetCanvas() helper as long as Application statics are not used
70+
71+
**c) Integration Tests (CANNOT be parallelized):**
72+
- Tests using [AutoInitShutdown]
73+
- Tests using Application.Begin, Application.RaiseKeyDownEvent, or other Application static methods
74+
- Tests that validate component behavior within full Application context
75+
- Tests that require ConfigurationManager or Application.Navigation
76+
77+
### 3. View/Adornment and View/Draw Tests
78+
**Finding:** After analyzing these tests, they all use [SetupFakeDriver] and test View.Draw() with visual verification. These are integration tests that validate how adornments render within the View system. They correctly belong in UnitTests.
79+
80+
**Recommendation:** Do NOT migrate these tests. They are integration tests by design and require the full Application/Driver context.
81+
82+
## Test Results
83+
84+
### UnitTests.Parallelizable
85+
- **Before:** 9,360 tests passing
86+
- **After:** 9,395 tests passing (+35)
87+
- **Result:** ✅ All tests pass
88+
89+
### UnitTests
90+
- **Status:** 3,488 tests passing (unchanged)
91+
- **Result:** ✅ No regressions
92+
93+
## Recommendations for Future Work
94+
95+
### 1. Continue Focused Migration
96+
97+
**Tests CAN be parallelized if they:**
98+
- ✅ Test properties, constructors, and basic operations
99+
- ✅ Use [SetupFakeDriver] without Application statics
100+
- ✅ Call View.Draw(), LayoutAndDraw() without Application statics
101+
- ✅ Verify visual output with DriverAssert (when using [SetupFakeDriver])
102+
- ✅ Create View hierarchies without Application.Top
103+
- ✅ Test events and behavior without global state
104+
105+
**Tests CANNOT be parallelized if they:**
106+
- ❌ Use [AutoInitShutdown] (requires Application.Init/Shutdown global state)
107+
- ❌ Set Application.Driver (global singleton)
108+
- ❌ Call Application.Init(), Application.Run/Run<T>(), or Application.Begin()
109+
- ❌ Modify ConfigurationManager global state (Enable/Load/Apply/Disable)
110+
- ❌ Modify static properties (Key.Separator, CultureInfo.CurrentCulture, etc.)
111+
- ❌ Use Application.Top, Application.Driver, Application.MainLoop, or Application.Navigation
112+
- ❌ Are true integration tests testing multiple components together
113+
114+
**Important Notes:**
115+
- Many tests blindly use the above when they don't need to and CAN be rewritten
116+
- Many tests APPEAR to be integration tests but are just poorly written and can be split
117+
- When in doubt, analyze if the test truly needs global state or can be refactored
118+
119+
### 2. Documentation
120+
Update test documentation to clarify:
121+
- **UnitTests** = Integration tests that validate components within Application context
122+
- **UnitTests.Parallelizable** = Pure unit tests with no global state dependencies
123+
- Provide examples of each type
124+
125+
### 3. New Test Development
126+
- Default to UnitTests.Parallelizable for new tests unless they require Application/Driver
127+
- When testing rendering, create both:
128+
- Pure unit test (properties, behavior) in Parallelizable
129+
- Rendering test with [SetupFakeDriver] can also go in Parallelizable (as long as Application statics are not used)
130+
- Integration test (Application context) in UnitTests
131+
132+
### 4. Remaining Category A Tests
133+
**Status:** Can be re-evaluated for migration
134+
135+
**Rationale:**
136+
- View/Adornment/* tests (19 tests): Use [SetupFakeDriver] and test View.Draw() - CAN be migrated if they don't use Application statics
137+
- View/Draw/* tests (32 tests): Use [SetupFakeDriver] and test rendering - CAN be migrated if they don't use Application statics
138+
- Need to analyze each test individually to check for Application static dependencies
139+
140+
## Conclusion
141+
142+
This migration successfully identified and moved 52 tests (35 Category A + 17 Views) to UnitTests.Parallelizable.
143+
144+
**Key Discovery:** Tests with [SetupFakeDriver] CAN run in parallel as long as they avoid Application statics. This significantly expands the scope of tests that can be parallelized beyond just property/constructor tests to include rendering tests.
145+
146+
The approach taken was to:
147+
1. Identify tests that don't use Application.Begin, Application.RaiseKeyDownEvent, Application.Navigation, or other Application static members
148+
2. Keep [SetupFakeDriver] tests that only use View.Draw() and DriverAssert
149+
3. Move [AutoInitShutdown] tests only if they can be rewritten to not use Application.Begin
150+
151+
**Migration Rate:** 52 tests migrated so far. Many more tests with [SetupFakeDriver] can potentially be migrated once they're analyzed for Application static usage. Estimated ~3,400 tests remaining to analyze.

‎Tests/UnitTests/Drawing/LineCanvasTests.cs‎

Lines changed: 0 additions & 163 deletions
Original file line numberDiff line numberDiff line change
@@ -299,169 +299,6 @@ string expected
299299
v.Dispose ();
300300
}
301301

302-
[InlineData (
303-
0,
304-
0,
305-
0,
306-
0,
307-
0,
308-
1,
309-
1
310-
)]
311-
[InlineData (
312-
0,
313-
0,
314-
1,
315-
0,
316-
0,
317-
1,
318-
1
319-
)]
320-
[InlineData (
321-
0,
322-
0,
323-
2,
324-
0,
325-
0,
326-
2,
327-
2
328-
)]
329-
[InlineData (
330-
0,
331-
0,
332-
3,
333-
0,
334-
0,
335-
3,
336-
3
337-
)]
338-
[InlineData (
339-
0,
340-
0,
341-
-1,
342-
0,
343-
0,
344-
1,
345-
1
346-
)]
347-
[InlineData (
348-
0,
349-
0,
350-
-2,
351-
-1,
352-
-1,
353-
2,
354-
2
355-
)]
356-
[InlineData (
357-
0,
358-
0,
359-
-3,
360-
-2,
361-
-2,
362-
3,
363-
3
364-
)]
365-
[Theory]
366-
[SetupFakeDriver]
367-
public void Viewport_H_And_V_Lines_Both_Positive (
368-
int x,
369-
int y,
370-
int length,
371-
int expectedX,
372-
int expectedY,
373-
int expectedWidth,
374-
int expectedHeight
375-
)
376-
{
377-
var canvas = new LineCanvas ();
378-
canvas.AddLine (new (x, y), length, Orientation.Horizontal, LineStyle.Single);
379-
canvas.AddLine (new (x, y), length, Orientation.Vertical, LineStyle.Single);
380-
381-
Assert.Equal (new (expectedX, expectedY, expectedWidth, expectedHeight), canvas.Bounds);
382-
}
383-
384-
[InlineData (
385-
0,
386-
0,
387-
0,
388-
0,
389-
0,
390-
1,
391-
1
392-
)]
393-
[InlineData (
394-
0,
395-
0,
396-
1,
397-
0,
398-
0,
399-
1,
400-
1
401-
)]
402-
[InlineData (
403-
0,
404-
0,
405-
2,
406-
0,
407-
0,
408-
2,
409-
1
410-
)]
411-
[InlineData (
412-
0,
413-
0,
414-
3,
415-
0,
416-
0,
417-
3,
418-
1
419-
)]
420-
[InlineData (
421-
0,
422-
0,
423-
-1,
424-
0,
425-
0,
426-
1,
427-
1
428-
)]
429-
[InlineData (
430-
0,
431-
0,
432-
-2,
433-
-1,
434-
0,
435-
2,
436-
1
437-
)]
438-
[InlineData (
439-
0,
440-
0,
441-
-3,
442-
-2,
443-
0,
444-
3,
445-
1
446-
)]
447-
[Theory]
448-
[SetupFakeDriver]
449-
public void Viewport_H_Line (
450-
int x,
451-
int y,
452-
int length,
453-
int expectedX,
454-
int expectedY,
455-
int expectedWidth,
456-
int expectedHeight
457-
)
458-
{
459-
var canvas = new LineCanvas ();
460-
canvas.AddLine (new (x, y), length, Orientation.Horizontal, LineStyle.Single);
461-
462-
Assert.Equal (new (expectedX, expectedY, expectedWidth, expectedHeight), canvas.Bounds);
463-
}
464-
465302
[Fact]
466303
[SetupFakeDriver]
467304
public void Viewport_Specific ()

‎Tests/UnitTests/Drawing/RulerTests.cs‎

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,6 @@ public class RulerTests
88
private readonly ITestOutputHelper _output;
99
public RulerTests (ITestOutputHelper output) { _output = output; }
1010

11-
[Fact]
12-
public void Attribute_set ()
13-
{
14-
var newAttribute = new Attribute (Color.Red, Color.Green);
15-
16-
var r = new Ruler ();
17-
r.Attribute = newAttribute;
18-
Assert.Equal (newAttribute, r.Attribute);
19-
}
20-
21-
[Fact]
22-
public void Constructor_Defaults ()
23-
{
24-
var r = new Ruler ();
25-
Assert.Equal (0, r.Length);
26-
Assert.Equal (Orientation.Horizontal, r.Orientation);
27-
}
28-
2911
[Fact]
3012
[AutoInitShutdown]
3113
public void Draw_Default ()
@@ -157,22 +139,4 @@ public void Draw_Vertical ()
157139
_output
158140
);
159141
}
160-
161-
[Fact]
162-
public void Length_set ()
163-
{
164-
var r = new Ruler ();
165-
Assert.Equal (0, r.Length);
166-
r.Length = 42;
167-
Assert.Equal (42, r.Length);
168-
}
169-
170-
[Fact]
171-
public void Orientation_set ()
172-
{
173-
var r = new Ruler ();
174-
Assert.Equal (Orientation.Horizontal, r.Orientation);
175-
r.Orientation = Orientation.Vertical;
176-
Assert.Equal (Orientation.Vertical, r.Orientation);
177-
}
178142
}

0 commit comments

Comments
 (0)