Enhances Prism.Maui navigation test coverage#3397
Merged
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be532fd391
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e202fa8 to
9e23a83
Compare
Adds extensive unit tests for `PageNavigationService` and `INavigationBuilder` extensions, covering various failure paths and edge cases to improve robustness. Refactors core navigation test infrastructure, including container, application, window manager, and page accessor mocks, to enable more comprehensive and accurate testing. Excludes `async void Navigate` extension methods from code coverage, clarifying their role as UI entry points not intended for direct unit testing.
Improves `TabbedPage` selected tab resolution by matching registered view names alongside CLR type names, addressing scenarios where they diverge. Refactors `PageNavigationService` unit tests for increased reliability: - Converts `async void` tests to `async Task`. - Explicitly asserts `INavigationResult` for navigation outcomes. - Introduces a shared `PageNavigationEventRecorder` for consistent tracking of page lifecycle events across DI-resolved instances. - Adds helper methods for navigating complex visual trees (e.g., finding tabbed or flyout pages). Updates `EventToCommandBehavior` tests to use `CollectionView` and a more flexible mock for event raising, ensuring accurate behavior simulation.
Assign PageNavigationContainerMock.SharedNavigationRecorder from PageNavigationServiceMock so DI-resolved pages record lifecycle events. Update NavigateAsync_From_NavigationPage_When_NotClearNavigationStack_And_SamePage to assert the Prism re-navigation sequence on the existing top page; the prior recorder.IsEmpty check passed only because resolved pages had no recorder.
Ensure region hosts inside region guests receive Parent so nested regions register correctly after layout (GitHub #3332). Add DryIoc region fixture test with explicit measure/arrange for nested ContentView regions.
Remove the dialog from IDialogContainer.DialogStack before invoking the completion callback so callers can navigate immediately after the dialog closes (GitHub #3395). Add DryIoc regression test covering GoBackAsync after ShowDialogAsync.
Issue3332_NestedContentRegion_InnerRegionReceivesGuest_AfterLayout now uses RequestNavigate for the outer region and relies on MockOuterGuestViewModel.OnNavigatedTo to navigate the inner region, matching the e2e sample. Mock inner VM implements IRegionAware for navigation target resolution.
DialogContainerPage runs PushModalAsync/PopModalAsync and DialogStack updates via IDispatcher.DispatchAsync when dispatch is required; run inline when already on the UI thread or when no dispatcher (avoids nested DispatchAsync deadlock during re-entrant close). Consolidated null/IsDispatchRequired guard per review.
Changes `GlobalPackageReference` to `PackageReference` in `Directory.Build.props` for several packages. This ensures consistent package referencing when Central Package Management (CPM) is enabled, as `GlobalPackageReference` is primarily used for version definitions within `Directory.Packages.props`.
9e23a83 to
0abb611
Compare
Replaces traditional .sln files with .slnx solution filter files for all e2e samples and the main library. This enables more flexible solution management, allowing developers to load specific subsets of projects to improve IDE performance and reduce clutter.
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
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.
Description of Change
This pull request significantly enhances the test coverage for Prism.Maui's navigation system, focusing on both the
PageNavigationServiceandINavigationBuilderextensions. It introduces new test fixtures to cover various success and failure paths, edge cases, and validation logic, ensuring robustness across different navigation scenarios.To facilitate these comprehensive tests, several mocking classes and test infrastructure components have been refactored or added. Additionally,
async voidnavigation methods withinNavigationBuilderExtensionsare now explicitly excluded from code coverage, acknowledging their role as fire-and-forget UI entry points, with testableNavigateAsynccounterparts already covered.Bugs Fixed
API Changes
Added:
System.Diagnostics.CodeAnalysisnamespace usage[ExcludeFromCodeCoverage]attribute toINavigationBuilder.Navigateextension methodsChanged:
NavigationPoprecord'sPageproperty type to nullablePage?Behavioral Changes
async void Navigateextension methods are now marked for exclusion from code coverage, improving internal code quality metrics. This does not alter the functional behavior for consumers of the library.PR Checklist