-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[cdac] Simplify usage of TestPlaceholderTarget in tests #109873
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
[cdac] Simplify usage of TestPlaceholderTarget in tests #109873
Conversation
Remove subclasses - squash
…ntext for it squash - mark created
Tagging subscribers to this area: @tommcdon |
@@ -13,6 +14,7 @@ internal class ExecutionManagerTestBuilder | |||
{ | |||
public const ulong ExecutionManagerCodeRangeMapAddress = 0x000a_fff0; | |||
|
|||
const bool UseFunclets = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? A const bool
seems odd to me. If this is important, let's add a comment what this is trying to capture.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm missing why that is odd? It's just indicating that the test builder (currently) always specifies to use funclets and it is not configurable on the instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll move it to something local in a follow-up,.
src/native/managed/cdacreader/tests/MockDescriptors/MockDescriptors.HashMap.cs
Outdated
Show resolved
Hide resolved
- Make `TestPlaceholderTarget` (mock target) constructor take type infos and global values - Handle reading of global values in mock target implementation - Use Moq for creating a `ContractRegistry` instead of our own explicit implementation - Remove subclasses of `TestPlaceholderTarget`
TestPlaceholderTarget
(mock target) constructor take type infos and global valuesContractRegistry
instead of our own explicit implementationTestPlaceholderTarget