Test switch scopes should read switch default values dynamically at initialization time #12490
Conversation
| // Provides default values for switches if they're not always false by default | ||
| private static bool GetSwitchDefaultValue(string switchName) | ||
| { | ||
| if (switchName == "Switch.System.Runtime.Serialization.SerializationGuard") |
There was a problem hiding this comment.
THese switches are not owned by us, and this method is not responsible for setting their defaults.
There was a problem hiding this comment.
The only 2 switches in this assembly are "false" by default. THis file is in a common folder, but is used only by System.Drawing.Common, we could either make is shared between winforms and S.D.C, but I don't see benefit in doing this. I'd rather move it under the S.D.C root
Tanya-Solyanik
force-pushed
the
switch-scopes
branch
from
7339590
to
0b483e2
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12490 +/- ##
===================================================
- Coverage 75.73525% 75.72766% -0.00759%
===================================================
Files 3153 3151 -2
Lines 635807 635802 -5
Branches 46975 46978 +3
===================================================
- Hits 481530 481478 -52
- Misses 150842 150895 +53
+ Partials 3435 3429 -6
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
|
||
| private void ResetLocalSwitches() | ||
| { | ||
| _testAccessor.s_anchorLayoutV2 = 0; |
There was a problem hiding this comment.
This class modifies statics and is not used in our tests. In fact we have alternatives to it - we can set specific switches via their specific scopes instead of changing the Target Framework name property.
| @@ -91,48 +91,47 @@ private static bool GetSwitchValue(string switchName, ref int cachedSwitchValue) | |||
| { | |||
| cachedSwitchValue = isSwitchEnabled ? 1 /*true*/ : -1 /*false*/; | |||
| } | |||
| else if (!hasSwitch) | |||
There was a problem hiding this comment.
This code is executed in the test codepath, when "TestSwitch.LocalAppContext.DisableCaching" is on, when we access the switch value for the first time in the process. Setting switch in the AppContext dictionaries will ensure that the correct value is read in AppContext.TryGetSwitch call. This is needed for tests that run the same scenario with different switch values.
There was a problem hiding this comment.
Why is this not a comment in the code?
There was a problem hiding this comment.
This is a description of one of the issues fixed in this PR, the comment was intended to highlight this fix among other changes. All it does is describe exactly what the code is doing, I don't see any value in it as a code comment.
This enables turning AppContext switches on and off within the unit test regardless of the default value
Microsoft Reviewers: Open in CodeFlow