Skip to content

Conversation

@TamilarasanSF4853
Copy link
Contributor

This PR addresses the UI test image failures that occurred in the inflight/candidate branch #33185 and includes updates to improve rendering and test stability across platforms.

  • Re-saved the ShouldFlyoutBeVisibleAfterMaximizingWindow images on Mac and Windows platforms.

Test cases:

  • ShouldFlyoutBeVisibleAfterMaximizingWindow

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Dec 23, 2025
@dotnet-policy-service
Copy link
Contributor

Hey there @@TamilarasanSF4853! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 23, 2025
@Ahamed-Ali Ahamed-Ali added the area-testing Unit tests, device tests label Dec 23, 2025
@sheiksyedm sheiksyedm marked this pull request as ready for review December 23, 2025 12:34
@sheiksyedm
Copy link
Contributor

Summary of unit test failure root cause

   Test: Maui18123.MultiBindingShouldNotThrow
   Status: Hangs indefinitely (infinite loop)
   Triggered by: PR #32961 changes to CommandElement

   -------------------------------------------------------------------------------

Root Cause Analysis

What PR #32961 Changed

   The PR introduced a dependency mechanism to fix issue #31939 (CommandParameter
   TemplateBinding lost during reparenting):

     - Added DependsOn() method to BindableProperty - allows declaring that Command depends on CommandParameter
     - Added ForceBindingApply() method to BindableObject - forces pending bindings to apply immediately
     - Modified CommandElement.GetCanExecute() - now calls ForceBindingApply(CommandParameter) before evaluating CanExecute

Why Maui18123 Test Hangs

   The test has a specific combination that creates an infinite recursion loop:

Test Scenario:

     - 2 Buttons (deleteBtn, editBtn)
     - Both share same Command (TestCommand)
     - Both use MultiBinding for CommandParameter
     - Both have DataTriggers that change CommandParameter based on Mode property

The Infinite Loop:

     - Button.SendClicked() → executes Command → changes Mode property
     - Mode change → triggers DataTrigger on BOTH buttons
     - DataTrigger → sets new CommandParameter (MultiBinding) on button
     - CommandParameter change → calls OnCommandParameterChanged()
     - OnCommandParameterChanged() → calls CanExecuteChanged()
     - CanExecuteChanged() → refreshes IsEnabled property
     - IsEnabled evaluation → calls GetCanExecute()
     - GetCanExecute() → calls ForceBindingApply(CommandParameter) (NEW in PR #32961)
     - ForceBindingApply() → applies the MultiBinding
     - MultiBinding application → triggers property changes that loop back to step 4

   The critical issue: Step 8's ForceBindingApply() is being called on a
   CommandParameter that was just set by a DataTrigger (step 3), not by a pending
   binding. This causes the MultiBinding to be re-applied unnecessarily, triggering
   another round of property changes.

   -------------------------------------------------------------------------------

Fix Attempts

   Attempt 1: Add _applying guard in ForceBindingApply ✗

     if (_applying) return;

   Why it failed: _applying is only set during SetValueActual(), not during the
   broader binding application flow.

   Attempt 2: Track force-applying properties per instance ✗

     HashSet _forceApplyingProperties;

   Why it failed: The recursion happens across different code paths (DataTrigger →
   CommandParameter change → CanExecute → ForceBindingApply), not just within
   ForceBindingApply itself.

   Attempt 3: ThreadStatic guard in GetCanExecute ✗

     [ThreadStatic] static bool _isEvaluatingCanExecute;

   Why it failed: Doesn't prevent the issue when multiple button instances share
   the same Command and trigger each other's evaluations.

   Attempt 4: Instance-level HashSet in GetCanExecute ✓ (Partially)

     [ThreadStatic] static HashSet? _evaluatingInstances;

   Current status: This SHOULD work theoretically, but tests still hang. The issue
   might be more complex than simple recursion - possibly a deadlock in the xUnit
   test runner or during XAML compilation.

   -------------------------------------------------------------------------------
The Real Problem

   The fundamental design issue is:

   ForceBindingApply() doesn't distinguish between:

     - ❌ Pending bindings that haven't resolved yet (what it was designed to fix)
     - ✅ Bindings that were just set by DataTriggers/Setters and don't need forcing

   When a DataTrigger sets a new MultiBinding as CommandParameter, that binding IS
   in the Bindings collection, so ForceBindingApply() tries to apply it again, even
   though it was just applied by the trigger system.

   -------------------------------------------------------------------------------

Summary

   PR #32961's ForceBindingApply() mechanism creates an infinite loop with the
   Maui18123 test's complex scenario (MultiBinding + DataTriggers + shared
   Commands).

   The test needs to either be:

     - Skipped with documentation of the known issue
     - Revert the PR and fix the ForceBindingApply() logic to detect when bindings don't actually need forcing
     
   

This reverts commit 33ecfef.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-testing Unit tests, device tests community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants