Miscellaneous MetadataUpdater cleanups#54751
Merged
marek-safar merged 3 commits intodotnet:mainfrom Jun 26, 2021
Merged
Conversation
MetadataUpdater.GetCapabilities() on CoreCLR always returns the supported edits, even if in the current running instance the environment variables for hot reload or EnC are unset. This change makes mono behave the same, except in the case where the runtime is built without hot reload support at all. MetadataUpdater.IsSupported still behaves the same as before at runtime - it returns true if there is some possibility that this ApplyUpdate could be called and could succeed (ie env is set apropriately)
Use PlatformDetection more, and add some utility conditions Add a new test for MetadataUpdater.IsSupported for scenarios where the test is launched with the environment setup appropriately - rather than where we would rely on the remote executor to set it for a child process.q
Instead of making the entire class a [ConditionalClass]. This lets us run the other tests (that validate parameters, or otherwise don't rely on actually attempting to apply updates) on more configurations Also slightly improves the xunit results report: [ConditionalClass] doesn't increment the Skipped test count, whereas [ConditionalFact] does.
mikem8361
approved these changes
Jun 25, 2021
stephentoub
approved these changes
Jun 25, 2021
ghost
locked as resolved and limited conversation to collaborators
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On Mono make
MetadataUpdater.GetCapabilities()return"Baseline"even if the environment isn't set up for applying updates (ie, no debugger, or the modifiable assemblies env var isn't set). This brings us in line with CoreCLR where that function always returns the same string. On Mono it now returns "Baseline" if the runtime has the hot_reload component, or "" if it doesn't (and thus applying updates is completely compiled out).Add a new test that checks that
MetadataUpdater.IsSupportedreturnstruefor configurations where the launch environment sets up the modifiable assemblies environment var (so: mobile and wasm). The platforms that can use the remote executor, the env var is not set, soMetadataUpdater.IsSupportedreturnsfalse.Replace the
[ConditionalClass]attribute on theApplyUpdateTestsuite with[ConditionalFact]on the individual tests. This lets the other tests (that validate argument checking, or otherwise don't actually need to attempt to apply updates) run on more configurations.