Fix / refactor FunctionsHostingConfigOptionsTest #10890
Open
+131
−181
Conversation
This file contains 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
fabiocav
reviewed
Feb 27, 2025
| // Note: For legacy purposes (we used to call Configuration.Bind() on this object), some properties whose ScmHostingConfig key and | ||
| // property name match exactly need to support "True/False". | ||
| // It is recommended that new properties only look at "1/0" for their setting. | ||
| yield return new object[] { nameof(FunctionsHostingConfigOptions.DisableLinuxAppServiceExecutionDetails), "DisableLinuxExecutionDetails=1", true }; |
There was a problem hiding this comment.
Can we use collection expressions here to reduce some of the verbosity (e.g. yield return [nameof(FunctionsHostingConfigOptions.DisableLinuxAppServiceExecutionDetails), "DisableLinuxExecutionDetails=1", true];)
| } | ||
|
|
||
| [Fact] | ||
| public void Inject_Succeded() |
There was a problem hiding this comment.
spelling: (previous issue) -> Inject_Succeeded
| { | ||
| (nameof(FunctionsHostingConfigOptions.DisableLinuxAppServiceExecutionDetails), "DisableLinuxExecutionDetails=1", true), | ||
| (nameof(FunctionsHostingConfigOptions.DisableLinuxAppServiceLogBackoff), "DisableLinuxLogBackoff=1", true), | ||
| // make sure all props are internal to prevent inadverntent binding in the future |
| .Where(p => p.Name != nameof(FunctionsHostingConfigOptions.Features)); | ||
|
|
||
| foreach (var prop in props) | ||
| var prop = AllProperties.Select(x => x[0] as PropertyInfo).Single(p => p.Name == propertyName); |
There was a problem hiding this comment.
p.Name == propertyName -> string.Equals with an explicit string comparison.
| [InlineData("feature1", "value1")] | ||
| [InlineData("featuree1", null)] | ||
| public async Task Case_Insensitive(string key, string expectedValue) | ||
| public static IEnumerable<object[]> AllProperties |
There was a problem hiding this comment.
Can we replace this with:
public static IEnumerable<PropertyInfo> AllProperties =>
typeof(FunctionsHostingConfigOptions).GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
.Where(p => p.Name != nameof(FunctionsHostingConfigOptions.Features));
public static IEnumerable<object[]> AllPropertiesInput => AllProperties.Select(p => new[] { p });Doing so would enable you to use AllPropertiesInput in the MemberData while changing the property match in line 151 with:
var prop = AllProperties.Single(p => string.Equals( p.Name, propertyName, StringComparison.Ordinal));
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue describing the changes in this PR
resolves #10889
Pull request checklist
IMPORTANT: Currently, changes must be backported to the
in-procbranch to be included in Core Tools and non-Flex deployments.in-procbranch is not required -- TODOrelease_notes.mdAdditional information
Refactors
FunctionsHostingConfigOptionsTestto stabilize and improve the tests.ConfigureDefaultTestWebScriptHostusageProperty_Validationtest into two, one for validating access and another for validating expectations. Also converts these into a theory usingMemberData.