blairconrad / SelfInitializingFakes

Like Fowler's self-initializing fakes.
MIT License
11 stars 3 forks source link

Add a builtin JSON serializer #80

Open blairconrad opened 4 years ago

blairconrad commented 4 years ago

Split from #79, as proposed by @CableZa.

We currently have support for XML and Binary serialization only.

blairconrad commented 4 years ago

@CableZa, can you explain why you need this functionality? My thinking is that from the end-user's perspective, the serialization format is not really relevant, so long as objects serialize and deserialize. Is there a particular use case that, for example, the XML serializer isn't meeting? If so, an alternative may be to work on that serializer rather than introduce a new builtin serializer that brings additional package dependencies.

blairconrad commented 4 years ago

Oh, and I should've mentioned before, it's always possible for clients to plug in their own serializers, so it's possible (in the interim, or forever) to benefit from a custom serializer without building it into the main package.

CableZa commented 4 years ago

@blairconrad I think I need to rewind my commits to see exactly why, but it may have had something to do with the support for Task<>.

I will see if I can get it working with the XML/Binary serializer.

Otherwise I just have a personal preference for JSON over XML, the recording files then matchup closer to what the real API's return.

CableZa commented 4 years ago

@blairconrad I see now why the builtin serializers didnt work for me, because they require the Serializable attribute is added to the input/output classes for the method being recorded.

The Newtonsoft JSON serializer doesn't have this requirement... I'm curious how this doesn't affect your usage - Are you adding the attribute to all your classes or using built-in/primitive types?

blairconrad commented 4 years ago

Hi, @CableZa.

For actual "production usage", the only interface I fake is

public interface IComicFetcher
{
    string GetContent(Uri url);
}

So, not all primitives, but Uri is serializable. As you've seen in the tests, other classes such as DateTime and Guid. The latter isn't serializable, but for some reason it seems to work anyhow. I'm not entirely sure why.

You're serializing a lot of custom classes? Earlier you'd said

have a personal preference for JSON over XML, the recording files then matchup closer to what the real API's return.

so I imagined (just imagined) that you were mostly dealing with services that returned stringy data that were already serialized JSON or the like.

CableZa commented 4 years ago

The services do indeed return JSON strings representing some DTO, but the code where I'm faking/recording the dependency expects the deserialized type.

I have apicontrollers that take in one or more of these services via constructor injection:

public class MyController : APIController
{
  public MyController(IExternalAPI externalAPI)
  {
     this.externalAPI = externalAPI;
   }

   public async Task<IHttpActionResult> GetStuff(int stuffId)
   {
     var stuffFromExternalAPI = externalAPI.GetStuff(stuffId);
     return Ok(stuffFromExternalAPI);
   }
}

public interface IExternalAPI 
{
   Stuff GetStuff(int stuffId);
}

public class Stuff 
{
  public int Id {get;set;}
  public MoreDetails NestedInfo {get;set;}
}

So I'm keen to use SelfInitializingFakes to allow testing 'MyController' with a faked/recorded IExternalAPI.

blairconrad commented 4 years ago

That clears some things up, @CableZa. The example is really helpful.

You're encountering a gap because this is not the intended use of the library. The goal is to replace the remote calls, Quoting Fowler:

One of the classic cases for using a TestDouble is when you call a remote service. Remote services are usually slow and often unreliable, so using a double is a good way to make your tests faster and more stable.

I think I see why you'd try to self-initialize a fake IExternalAPI, but the intent is to fake out the actual remote service, not including any fancy rehydration logic. This has two advantages:

  1. the services' APIs tend to be simpler, e.g. returning serialized-as-JSON results, making the faking easier, and
  2. this keeps the rehydration logic outside of the fake boundary, so it is still tested. If you fake a class that already does the serialization (or interface to a class that does this), if you end up getting a response from the service that breaks your serialization, you might not be aware. Or you're not able to refactor the deserialization layer without a live service

I'm not unsympathetic to your plight, but I just can't yet support adding a JSON serializer for this use case. Especially the Newtonsoft JSON serializer, which would mean bringing in an additional dependency for all users, even if they don't care about JSON serialization.

CableZa commented 4 years ago

Thanks for clarifying @blairconrad. Interesting and makes sense to me, figured I was missing something regarding intended usage :) I'll see if I can use the library as intended and record at the lower layer (before deserialization happens), alternatively I'll use a JsonSerializer external to the SelfInitializingFakes library.

Out of interest do you use any similar approach to record & playback database calls?

blairconrad commented 4 years ago

Out of interest do you use any similar approach to record & playback database calls?

I've never done. Of course, I only use SelfInitializingFakes for one (toy) project. I imagine it would technically work, but I'm not sure it's needed. You'd be trading a database call to a read from the filesystem (I assume). Like remote services, databases can be a little slower than other operations, but generally I'd think they'd be more reliable, so I'm not sure there's that much benefit.