Skip to content

Add test coverage for public API surface of MouseActionConverter #9891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 22, 2025

Conversation

h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Oct 3, 2024

Description

Adds test coverage for MouseActionConverter. I've also fixed an unintentional behavioral change I've introduced with #9676, which would mean it wouldn't throw InvalidEnumArgumentException due to passing wrong value type to the exception.

  • As I've outlined previously, there are basically two (in my opinion) bugs in the class
    • CanConvertTo throws InvalidCastException due to an unboxing cast when you pass an int
    • The same thing happens in ConvertTo when you pass an int within range

Customer Impact

Improved test coverage on public surface.

Regression

Fixing my own oversight at the same time.

Testing

Local build.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@dipeshmsft
Copy link
Member

As I've outlined previously, there are basically two (in my opinion) bugs in the class

  • CanConvertTo throws InvalidCastException due to a cast when you pass an int
  • The same thing happens in ConvertTo when you pass an int within range

Good point, this looks like an issue in the class implementation.

Copy link
Member

@dipeshmsft dipeshmsft left a comment

Choose a reason for hiding this comment

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

Rest of the changes look good to me.

@h3xds1nz h3xds1nz force-pushed the add-mouseaction-tests branch from c3d0283 to 085c76e Compare January 22, 2025 10:38
@h3xds1nz
Copy link
Member Author

h3xds1nz commented Jan 22, 2025

@dipeshmsft Added a bit more cases as well and converted to file-scoped as with all my newer tests (will do that bit for TextDecorations as well). It's rebased btw so I had to force push.

@dipeshmsft dipeshmsft merged commit e145850 into dotnet:main Jan 22, 2025
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 22, 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.

2 participants