Consolidate all the test handler construction#16200
Conversation
| var handler = Activator.CreateInstance<TCustomHandler>(); | ||
| var handler = mauiContext.Handlers.GetHandler(element.GetType()); |
There was a problem hiding this comment.
The is the actual change :)
Sometimes handlers were created with the activator (OG way) sometimes they are created from the handler mappers in the maui context (new way). This PR will make it always use the mapper.
The generic parameter on the CreateHandler is still useful as it casts the return type ... and we can validate the handler type.
| mauiAppBuilder.ConfigureMauiHandlers(handlers => | ||
| handlers.AddHandler<TStub, THandler>()); |
There was a problem hiding this comment.
This PR avoids the verbose code to add a handler to the mapper by making use of this new method that was added to clean up tests. Instead of using the activator randomly, we just always add the test's handler to the mapper.
This can be overridden by manually calling EnsureHandlerCreated and passing in the desired action.
| [Fact(DisplayName = "Context flyout creates expected WinUI elements")] | ||
| public async Task ContextFlyoutCreatesExpectedWinUIElements() | ||
| { | ||
| SetupBuilder(); |
There was a problem hiding this comment.
Another case that I am cleaning up is the proliferation of the SetupBuilder() calls. Using the nice (and existing) ConfigureBuilder method on the base, we can set up for all the tests in the class.
| public async Task AutoSizeInitializesCorrectly(EditorAutoSizeOption option) | ||
| { | ||
| EnsureHandlerCreated(builder => | ||
| builder.ConfigureMauiHandlers(handlers => | ||
| handlers.AddHandler<VerticalStackLayout, LayoutHandler>())); |
There was a problem hiding this comment.
And we can still add/override in an individual test.
| EnsureHandlerCreated(builder => | ||
| builder.ConfigureMauiHandlers(handlers => | ||
| handlers.AddHandler<EditorStub, CustomEditorHandler>())); |
There was a problem hiding this comment.
@rachelkang this is the fix that you suggested for #16116
Eilon
added
the
area-infrastructure
# Conflicts: # src/Controls/tests/DeviceTests/Elements/Border/BorderTests.cs # src/Controls/tests/DeviceTests/Elements/CheckBox/CheckBoxTests.cs # src/Controls/tests/DeviceTests/Elements/ContentView/ContentViewTests.cs # src/Controls/tests/DeviceTests/Elements/Editor/EditorTests.cs # src/Controls/tests/DeviceTests/Elements/Entry/EntryTests.cs # src/Controls/tests/DeviceTests/Elements/VisualElement/VisualElementTreeTests.cs
mattleibow
requested review from
PureWeen,
StephaneDelcroix,
hartez and
rachelkang
and removed request for
StephaneDelcroix
mattleibow
changed the title
samhouts
added
area-testing
samhouts
added
stale
|
Hi @mattleibow. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time. |
ghost
closed this
mattleibow
added
stale
|
closing this for now If this seems good to still get in for NET10, create a new PR :-) |
5 participants
Description of Change
While trying to write tests, I found that there are so many different ways to create handlers - each day we are adding a new layer - so this PR tries to remove the variations and attempts to create a more structured and predictable test setup.