Remove UI tests that only restate the code they cover - #3982
Open
siegfriedpammer wants to merge 1 commit into
Open
Remove UI tests that only restate the code they cover#3982siegfriedpammer wants to merge 1 commit into
siegfriedpammer wants to merge 1 commit into
Conversation
These fixtures were written while porting to Avalonia, as an author's own verification step rather than as coverage: reflection asserting that a type derives from its base and that a property has the type it is declared with; literals (MinHeight 29, Padding 3, MaxWidth 900) copied out of the .axaml beside them; a property override asserted only so pane descendants stay reachable from tests. None of them can fail except when someone deliberately edits the line they mirror, and then they fail as a chore. StartupPerfTests keeps its two [Explicit] benchmarks, which print per-phase timings worth reading. The third was a wall-clock assertion (8 CoreLib copies must settle in under 15s) that ran in CI, where a shared runner decides the verdict; as [Explicit] it would be strictly dominated by the 200-assembly benchmark it was derived from, so it goes. Two fixtures are trimmed rather than deleted, because their kernel is real: XmlDocLoader's ref-pack fallback has no other test in the repo, and the MenuIcon metadata rasterisation was dropped once during the port already. Both now assert that without booting MainWindow to reach it. This is worth about two seconds - it buys reviewers less to read, not CI less to do. Assisted-by: Claude:claude-opus-5:Claude Code
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
1 participant
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.
Stacked on #3981 (base branch
christophwille/nu0826), which removes the two 900-iterationprocess-list scroll tests. This is the follow-up sweep for the rest of the same category.
#3981 found the one test pair that was actually expensive. Auditing the rest of
ILSpy.Testsfor tests of the same kind turned up seven more that assert nothing a user could notice:
ContentPageHierarchyTests(deleted) - reflection asserting that four types derive fromtheir base and that
ContentTabPage.Contenthas the type it is declared with. The compilerenforces all three.
ToolPaneDeferredContentTests(deleted) - asserts a one-line property override returnsfalse; its own comment gives the reason as "without this, tests can't reach descendants ofa pane".
BookmarksPaneStructureTests- droppedMinHeight == 29/Padding == 3/BorderThickness == (0,0,0,1), copied out ofBookmarksPane.axaml. The cell-template testin the same fixture stays.
DocumentationRendererTooltipWidthTests- droppedMaxWidth >= 900, a constant pinnedby the change that chose it. The wrapping test stays.
StartupPerfTests- dropped the one non-[Explicit]test, which asserted that copyingCoreLib 8x and settling the list takes under 15s, i.e. let a shared CI runner decide the
verdict. Both
[Explicit]benchmarks are untouched; the deleted one was the "CI-runnablevariant" of one of them and has no purpose off CI.
Two are trimmed rather than deleted, because their kernel is real coverage:
XmlDocumentationTests-XmlDocLoader's modern-.NET ref-pack fallback has no othertest in the repo. Now a plain
[Test]that opens CoreLib throughAssemblyList, instead ofbooting
MainWindowand expandingSystem.Stringin the tree to reach the same call.MenuIconWiringProbe->MainMenuIconTests- the File > Open assertion guardsMEF-
MenuIcon-metadata toNativeMenuItem.Iconrasterisation, which the Avalonia portdropped once already. The surrounding menu walk and the "at least 5 leaves have an icon"
threshold are gone.
Cost
Worth stating plainly: about two seconds. These are not slow - they are noise. Measured
locally, the eight removed tests total 2.1s and the
XmlDocLoadertrim saves another 0.6s.The CI win in this area was #3981's, not this PR's.
For scale, from the same measurement run (1180 tests before #3981): the scroll pair was 618s of
1236s. Of the remaining 618s, 559s (90%) belongs to the 547 tests that boot the app or reach
into CoreLib/System.Linq, at a mean of 1.02s each; the 631 tests that stay off the BCL cost
59s total, 0.09s each. Moving heavy tests onto the existing
FixtureAssemblystub is wherethe remaining time is - that is a separate change.
Verification
ILSpy.Testson Linux: 1171 tests, 0 failures, 3 skipped ([Explicit]).