Skip to content

Conversation

@ThomasGoulet73
Copy link
Contributor

@ThomasGoulet73 ThomasGoulet73 commented Feb 28, 2025

Description

Fixes the test failure seen in main currently. This was caused by the switch from xUnit test runner to VSTest test runner introduced in #10514 which changed the order in which test were ran and exposed an existing bug in the tests.

The problem is because the xUnit test runner ran the test Register_InvokeStringTypeTypeValidateFail_Throws before Register_InvokeStringTypeType_Success and VSTest test runner runs them in inverse order. This causes problem because both test registers a dependency property for the same type with the same property name, the thing is that Register_InvokeStringTypeTypeValidateFail_Throws expects the registration to fail and not register the dependency property in the cache while Register_InvokeStringTypeType_Success expects the registration to succeed and register the dependency property in the cache.

When the order is Register_InvokeStringTypeTypeValidateFail_Throws -> Register_InvokeStringTypeType_Success it's fine because the dependency property is not in the cache for the second test.
When the order is Register_InvokeStringTypeType_Success -> Register_InvokeStringTypeTypeValidateFail_Throws it fails because Register_InvokeStringTypeTypeValidateFail_Throws fails early because the property is already registered for the second test.

Here's the 2 conflicting dependency property name + type:

yield return new object?[] { " ", typeof(int), typeof(DependencyObjectTests), 0 };

yield return new object?[] { " ", typeof(int), typeof(DependencyObjectTests), null, 0 };
yield return new object?[] { " ", typeof(int), typeof(DependencyObject), new PropertyMetadata(), 0 };

Side note:
We should probably refactor how tests are done for DependencyProperty, this PR is just a patch to fix the current failure. IMO we should have 1 type per tests that register dependency properties, if we don't want to have a mess of types we could possibly create the types at runtime using IL emit.

Customer Impact

None, tests only.

Regression

WPF tests regression only.

Testing

Running the tests locally.

Risk

Low to none, tests only.

Microsoft Reviewers: Open in CodeFlow

@ThomasGoulet73 ThomasGoulet73 requested review from a team as code owners February 28, 2025 04:13
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Feb 28, 2025
@ThomasGoulet73
Copy link
Contributor Author

@dotnet/wpf-developers: It would be a good idea to prioritize this to fix the test failures in main and unblock other PRs.

@dipeshmsft
Copy link
Member

LGTM

@codecov
Copy link

codecov bot commented Feb 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@009ae5a). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #10522   +/-   ##
==========================================
  Coverage        ?   10.94419%           
==========================================
  Files           ?        3215           
  Lines           ?      648472           
  Branches        ?       71534           
==========================================
  Hits            ?       70970           
  Misses          ?      576509           
  Partials        ?         993           
Flag Coverage Δ
Debug 10.94419% <100.00000%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

@singhashish-wpf
Copy link
Contributor

Thank you @ThomasGoulet73 ! 🙏

@singhashish-wpf singhashish-wpf merged commit 2b923a8 into dotnet:main Feb 28, 2025
8 checks passed
@ThomasGoulet73 ThomasGoulet73 deleted the fix-DependencyProperty-test branch February 28, 2025 04:44
@h3xds1nz
Copy link
Member

@ThomasGoulet73 #10323 looks like you got to it sooner haha

@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants