Can't implement ISettings

[bjorkstromm]

I'm using the NuGet client libraries in a application, and now I need to write some tests. I've previously mocked ISettings quite easily. But now, when updating from 4.7.0 to 4.9.1 I've run into a problem with ISettings.

The problem:

• I need to implement method SettingSection GetSection(string sectionName);

• SettingSection inherits from SettingsBase.

• [SettingsBase] defines an abstract internal method Clone()... https://github.com/NuGet/NuGet.Client/blob/release-4.9.1-rtm/src/NuGet.Core/NuGet.Configuration/Settings/SettingBase.cs#L53... I can't override that, because it's internal.

• I could have used VirtualSettingSection, which your tests use. It's a public class, but it only has internal constructors....

Please advice... How to implement ISettings and specifically SettingSection using NuGet Client libraries. Thank you!

[bjorkstromm]

I went the easy route and used reflection for creating an instance of VirtualSettingSection, see https://github.com/cake-build/cake/pull/2371/files#diff-16c125384e832bf918d13f23c42bd6c4.. There should be a better option for implementing ISettings...

[jainaashish]

@PatoBeltran can you help here?

[PatoBeltran]

hey @mholo65 I was looking into this, the idea is that if you want to implement ISettings, you should be able to implement the whole thing, and not necessarily take some of the decisions we used when doing our internal implementation.

I see that for your case you only want to mock it for testing purposes. From what I see there could be two options in how to solve this:

1. Update Clone() to be public instead of internal.
2. Update some of the internal constructors inVirtualSettingSection for people to use the same way we do in our tests.

I think the best solution out of the two would be the first one, and that way you could easily subclass SettingSection and easily mock it. I wouldn't want to do the second solution, since the VirtualSettingSection is just an implementation detail for our design and it doesn't make sense for people to be able to create this object when using our API. What do you think?

[bjorkstromm]

@PatoBeltran, thanks for looking into this. Option 1) making it public is probably the best option. Want me to send a PR?

[PatoBeltran]

@mholo65 if you can send a PR I would appreciate it!

[PatoBeltran]

hey @mholo65 I sent a PR to fix this, I would appreciate if you can look at it and see if it would address your scenario. Thanks for helping us improve our codebase!

[PatoBeltran]

PR has been merged, closing this issue.

[bjorkstromm]

@PatoBeltran thank you! This looks to solve the problem I was experiencing!