Add test coverage for public API surface of TextDecorationCollectionConverter

[h3xds1nz]

Description

Adds test coverage for TextDecorationCollectionConverter. Both the original and my PR #9778 have been tested against this.

• Again, this class is rather inconsistent with others.
  • Static ConvertFromString will return null on null but instance method will throw with NotSupportedException
  • While ConvertTo claims it only supports InstanceDescriptor as destinationType, it calls into base which will gladly accept string and based on the value type it will either return string.Empty (if value was null), or it returns the type name using ToString.
• I did not even include those edge cases because they should be rather fixed imho for more consistent behavior.

Customer Impact

Improved test coverage on public surface.

Regression

No.

Testing

Local build.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

[dipeshmsft]

@h3xds1nz here are a few things that I think we can do in this PR :

1. Add a test where ConvertTo runs successfully and for verification we can use the InstanceDescriptor to create an instance of the TextDecorationCollection class .
2. Add a test for ConvertFrom with input of type other than string to hit the other code paths of ConvertFrom method.

[dipeshmsft]

• Again, this class is rather inconsistent with others.
  - Static ConvertFromString will return null on null but instance method will throw with NotSupportedException
  - While ConvertTo claims it only supports InstanceDescriptor as destinationType, it calls into base which will gladly accept string and based on the value type it will either return string.Empty (if value was null), or it returns the type name using ToString.
• I did not even include those edge cases because they should be rather fixed imho for more consistent behavior.

Probably you can open one issue for these issues you found in this and the MouseActionConverter class.

[h3xds1nz]

@dipeshmsft I've added multitude of other tests, what I do not like is that its dependent now on InstanceDescriptor as a whole so its beyond the "unit" standard and verifies the ctor of TextDecorationCollection directly in the test.

It doesn't hurt but it is no longer fully "unit" in this case. Not that it really matters, some test is better than no-test, right.

#9778 guess it is time for this sad PR? 😀

[dipeshmsft]

Thanks @h3xds1nz for the quick resolution.

I understand that invoking InstanceDescriptor makes it less of a unit test and if there is a way to test the ConvertTo function without it, I am more than happy to have that.

[h3xds1nz]

@dipeshmsft We could technically just check the arguments match in the descriptor, but that's still not isolated.

I'd just do it (keep it) this way now. Can be always pulled away into an integration test later and only check for not null here and typeof(InstanceDescriptor).