Skip to content

Conversation

@bijington
Copy link
Contributor

Description of Change

This doesn't technically fix any bugs but it does fix some inconsistencies in our unit tests. You may notice there are some Console.Writeline statements in this PR. What I have observed on occasion is that Closing is logged but Closed is not which makes me wonder if it is close to potentially reproducing #2703

Linked Issues

  • Fixes #

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

Copilot AI review requested due to automatic review settings June 19, 2025 13:01

This comment was marked as outdated.

@TheCodeTraveler TheCodeTraveler requested a review from Copilot June 22, 2025 22:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the old AddPopup helper to avoid swallowed exceptions and modernizes the unit tests to use short- and long-lived popup classes that correctly close themselves. Key changes include:

  • Deleted the legacy internal AddPopup method from PopupService.shared.cs.
  • Replaced all uses of MockSelfClosingPopup/MockPageViewModel in tests with ShortLivedSelfClosingPopup/ShortLivedMockPageViewModel (and related long-lived variants).
  • Introduced new test popup types with configurable durations, added GC calls in open/close handlers, and updated the mock dispatcher to block for delays.

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

File Description
src/CommunityToolkit.Maui/Services/PopupService.shared.cs Removed deprecated internal AddPopup method.
src/CommunityToolkit.Maui.UnitTests/Mocks/MockDispatcher.cs Changed DispatchDelayed to use Thread.Sleep and return true.
src/CommunityToolkit.Maui.UnitTests/Layouts/StateContainerTests.cs Switched to Assert.ThrowsAnyAsync for animation timeout tests.
src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs Updated service registrations for the new popup/view-model types.

@TheCodeTraveler TheCodeTraveler merged commit f4fdb35 into main Jun 22, 2025
10 checks passed
@TheCodeTraveler TheCodeTraveler deleted the feature/sl-improve-popup-unit-tests branch June 22, 2025 23:20
@github-actions github-actions bot locked and limited conversation to collaborators Jun 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants