Skip to content

Replace the use of dynamic with Reflection to support AOT#269

Merged
iancooper merged 5 commits intoBrighterCommand:masterfrom
NetworkVisor:dev-removedynamic
Apr 7, 2025
Merged

Replace the use of dynamic with Reflection to support AOT#269
iancooper merged 5 commits intoBrighterCommand:masterfrom
NetworkVisor:dev-removedynamic

Conversation

@SteveBush
Copy link
Copy Markdown
Contributor

See issue #268
Replaced the use of (dynamic) with using reflection. Ahead Of Time compilation does not support runtime code generation, which is used by the cast for dynamic.

I added a Maui Test Application to validate the changes. It emulates SampleMinimalApi but from within an IOS, Android, WinUI, or MacCatalyst MAUI application. AOT compilation is only turned on for Release. AOT compilation also strips out all debugging information, so attaching a debugger is not supported. To fix something in MAUI, the general strategy is to get it working with unit tests first and then try it on the device.

I contributed some TestHelpers that I use in my personal development. A custom testcase base class, a logging provider that outputs to ITestOutputHelper, and their supporting methods.

Lastly, I made some design choices that I wanted to call out.

  1. Reflection generates a TargetInvocationException exception instead of a FormatException when called by Moq. I had the choice of fixing all of the tests to expect a TargetInvocationException exception (happy to do) or wrapping them with a FormatException so the existing tests don't fail. My preference would be to fix the tests but I went with the test compatible option.
  2. Added my own testing code. I'm happy to contribute this or remove it. I'm fine either way.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper
Copy link
Copy Markdown
Member

Thanks @SteveBush I'll take a look at some point today.

@iancooper iancooper added .NET Pull requests that update .net code enhancement 2 - In Progress labels Apr 3, 2025
Copy link
Copy Markdown
Member

@iancooper iancooper left a comment

Choose a reason for hiding this comment

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

Thanks @SteveBush I can't really comment on the Maui stuff, as I only scratch the surface there. But the move away from dynamic looks fine to me.

@iancooper
Copy link
Copy Markdown
Member

I need to update the GHA build and dependencies before Darker will build properly. It looks like we didn't update it for dotnet 9. I'll do that a bit later, and let you know once the build is green. That way you can merge and your build should go green

@SteveBush
Copy link
Copy Markdown
Contributor Author

I need to update the GHA build and dependencies before Darker will build properly. It looks like we didn't update it for dotnet 9. I'll do that a bit later, and let you know once the build is green. That way you can merge and your build should go green

Thanks. Before the merge, I would also like to make some changes:

  • Move the test helpers to a separate project so I can include them in other tests for future contributors.
  • Remove the wrapper for FormatException and fix the tests to assert on TargetInvocationException. The FormatException is just an artifact of using Moq. In my opinion, throwing TargetInvocationException is the right thing to do.

I will update the PR with these changes.

Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper iancooper merged commit 22cf26f into BrighterCommand:master Apr 7, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Done enhancement .NET Pull requests that update .net code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants