Skip to content
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

Remove reflection usage from Console tests #73224

Merged
merged 3 commits into from
Aug 2, 2022

Conversation

adamsitnik
Copy link
Member

In #72193 I've moved most of TermInfo.cs to separate files to be able to implement testable key parser without using the reflection.

In this PR I remove remaining reflection-based logic. This required referencing few code files, but IMO it's lesser evil compared to relying on reflection which can be easily broken.

fixes #73212

@ghost
Copy link

ghost commented Aug 2, 2022

Tagging subscribers to this area: @dotnet/area-system-console
See info in area-owners.md if you want to be subscribed.

Issue Details

In #72193 I've moved most of TermInfo.cs to separate files to be able to implement testable key parser without using the reflection.

In this PR I remove remaining reflection-based logic. This required referencing few code files, but IMO it's lesser evil compared to relying on reflection which can be easily broken.

fixes #73212

Author: adamsitnik
Assignees: -
Labels:

area-System.Console

Milestone: 7.0.0

@adamsitnik
Copy link
Member Author

/azp list

@adamsitnik
Copy link
Member Author

@MichalStrehovsky what do I need to do to run NativeAOT tests?

@@ -54,9 +53,32 @@
<ItemGroup Condition="'$(TargetPlatformIdentifier)' != 'windows'">
<Compile Include="CancelKeyPress.Unix.cs" />
<Compile Include="NonStandardConfiguration.Unix.cs" />
<Compile Include="TermInfo.Unix.cs" />
<Compile Include="..\src\System\TermInfo.cs" Link="src\System\TermInfo.cs" />
<Compile Include="..\src\System\TermInfo.DatabaseFactory.cs" Link="src\System\TermInfo.DatabaseFactory.cs" />
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, in other libraries, e.g. all of the System.Net.* libs, System.Text.RegularExpressions, etc., we've split the test projects into FunctionalTests and UnitTests, where the difference is the FunctionalTests are written only in terms of the public surface area exposed by the library under test, and the UnitTests include source from the target library in order to test internals directly.

@adamsitnik adamsitnik merged commit 1e7299d into dotnet:main Aug 2, 2022
@MichalStrehovsky
Copy link
Member

@MichalStrehovsky what do I need to do to run NativeAOT tests?

They're in the runtime-extra-platforms leg. But if there's no more reflection, this should be good! Thank you!

@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terminfo.terminfoverification
3 participants