Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.2k stars 4.55k forks source link

[BUG] No factory method for SearchAddressResultItem #40463

Open jlrosenlof-kr opened 8 months ago

jlrosenlof-kr commented 8 months ago

Library name and version

Azure.Maps.Search 1.0.0-beta.4

Describe the bug

The Unit Testing and Mocking page in the Azure SDK docs indicates that output model classes will have factory classes to instantiate them. I am trying to do some unit testing and I need to mock a SearchAddressResultItem. The constructors are all internal and the properties are all read only so I've got no way to mock this class. Additionally, there doesn't appear to be a factory class to instantiate this class, either. I have looked at the MapsSearchModelFactory and there are no factory methods which create a SearchAddressResultItem.

This seems like a pretty big oversight so I'm wondering if I'm missing something. Is there another factory class that I should be using?

Expected behavior

To be able to mock a SearchAddressResultItem class so that I can do unit testing.

Actual behavior

No way to create a SearchAddressResultItem class other than making an actual call to the Maps service.

Reproduction Steps

Try to call MapsSearchModelFactory and no factory method is listed.

Environment

.NET SDK (reflecting any global.json): Version: 6.0.319 Commit: e5c94b7d10

Runtime Environment: OS Name: Windows OS Version: 10.0.19045 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\6.0.319\

global.json file: Not found

Host: Version: 6.0.24 Architecture: x64 Commit: e7b8488daf

.NET SDKs installed: 1.1.7 [C:\Program Files\dotnet\sdk] 2.1.526 [C:\Program Files\dotnet\sdk] 2.2.106 [C:\Program Files\dotnet\sdk] 3.1.426 [C:\Program Files\dotnet\sdk] 5.0.408 [C:\Program Files\dotnet\sdk] 5.0.416 [C:\Program Files\dotnet\sdk] 6.0.300 [C:\Program Files\dotnet\sdk] 6.0.319 [C:\Program Files\dotnet\sdk]

.NET runtimes installed: Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.All 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All] Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.AspNetCore.App 6.0.24 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App] Microsoft.NETCore.App 1.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 1.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 2.2.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.NETCore.App 6.0.24 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App] Microsoft.WindowsDesktop.App 3.1.30 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App] Microsoft.WindowsDesktop.App 6.0.24 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

IDE: Visual Studio Code 1.84.2

ArcturusZhang commented 8 months ago

Hi @jlrosenlof-kr thanks for this issue. This is indeed currently a shortage in our code generator. We will fix this soon.

jlrosenlof-kr commented 7 months ago

Hi @ArcturusZhang thanks for getting back to me and for the information about the issue.

I wanted to get input/recommendations about a way forward in the meantime since it looks like a fix for this is potentially a couple of months away. I use NSubstitute for mocking in our unit tests. I have tried creating creating mocks of SearchAddressResultItem, SearchAddressResult, and several other classes which those classes depend on. Some of them have internal constructors and some of the types, like LatLongPairAbbreviated are completely internal; they are not publicly exposed.

Unfortunately, I can't wait a couple of months for this to be resolved. I have to move forward with something now and I would really to be able to unit test the code that I've created. Currently, I am using reflection to try to construct classes and types. That seems to be working but it is really cumbersome and time consuming to look up all of the type definitions and manually construct the classes. Is there a better or maybe faster way of mocking these classes or am I stuck with reflection if I want to be able to move forward now?

Also, one last thing that's sort of related but is also a separate thing. The decision to not use interfaces and instead use classes with virtual members is really cumbersome and makes unit testing more difficult. This issue here is just one example. We, the end user developers are stuck hoping that the Azure SDK team either remembers or has time to create factory classes for all of the classes that we might need. Then, if the team misses something or doesn't have time to create the factories we're stuck waiting for a while until the fix gets included and then works its way through the various validation processes. That just seems really prone to problems and failure. I saw another issue where somebody from the Azure SDK team indicated that this design decision was made to make versioning easier and because using interfaces locks in certain design decisions into the interface. I do understand that reasoning; I've dealt with that myself. However, the benefits of following the design principle to use interfaces yield themselves in various places like unit testing, dependency injection, etc. Not using the interfaces just makes things more brittle and difficult for us end users. I don't know if it would do any good to submit an issue or something in CodeVoice because it seems like this design decision is baked into the Azure SDK. Is it worth it to put in a request to reconsider this design decision? At the very least, please give us some workaround for the Output Model classes? Having them be completely internal with read only properties leaves us totally at the mercy of the Azure SDK team to create factory classes for all of them. If that doesn't happen, like in the case of this issue, we're stuck either waiting or using cumbersome reflection to be able to unit test our code.

jlrosenlof-kr commented 7 months ago

Hi @ArcturusZhang I found another issue with a different model factory. I'm not sure if I should open another issue or just put this in the same issue. I'm going to put it here for now and then I'll break it off later if necessary.

I need to create a unit test for some routing functionality. I was happy to see that the MapsRoutingModelFactory has a method for RouteData which will allow me to create a RouteData object. However, the method parameters take some types which themselves need MapsRoutingModelFactory methods: RouteSummary and RouteLeg. I see a factory method to create a RouteLegSummary but nothing for RouteSummary or RouteLeg. Is there something that I am missing? When the RouteData factory method asks for an IEnumerable<RouteLeg>, how am I supposed to create that without having a factory method for RouteLeg?