In unit testing: How to abstract a dependency of subject under test?

Question

Disclaimer: I am learning unit testing. I am also kind of beginner in object-oriented design.

Currently, I am involved in the development of an application to manage the finance of a humble food outlet. We are implementing an ASP.net API with Google spreadsheet as database.

Now I want to add unit tests (thinking of using either NUnit or XUnit.net) for the project. Then will proceed to refactor the code for better maintainability

Source Code:

// this class is a repository pattern implementation
// to encapsulate the logic to access the data
public class GoogleSheetsService
{
    public GoogleSheetsService(SheetsService sheetsService) {...}
    public IList<IList<object>> GetDataFromSheet(string spreadSheetID, string range) {...}
    public void AddtoSheet(string spreadSheetID, string range, IList<object>)  {...}
    public void UpdateSheet(string spreadSheetID, string range, IList<object>)  { ... }
    public void DeleteRow(string spreadSheetID, string range, int rowIndex) { ... }
    
}
public class MenuData
{
    public MenuData(string mastersheetid)
    {
      masterSheetId = mastersheetid;
    }
    public List<Menu> GetMenuList(GoogleSheetsService _service)
    {
        ..........
    }
}

GoogleSheetsService is a simple class to interact with Google spreadsheet by adding/ modifying the data.

To write a unit test for GetMenuList method, I need a test double to the GoogleSheetsService in order to isolate the SUT.

public interface IGoogleSheetsService
{
   public IList<IList<object>> GetDataFromSheet(string spreadSheetID, string range);
   public void AddtoSheet(string spreadSheetID, string range, IList<object>);
   public void UpdateSheet(string spreadSheetID, string range, IList<object>);
   public void DeleteRow(string spreadSheetID, string range, int rowIndex);
}

The only known way to do so to me is by extracting an interface from GoogleSheetsService class like below and use the interface to create a stub or mock using Moq framework (using mockObject.Setup(x => ...)). This blog post says having such Header Interfaces is not a good sign. My question is: help me to identify actual abstraction out of GoogleSheetsService

What is the better way to isolate the GoogleSheetsService class in unit testing ?

I understand that in the above-mentioned way, there is no actual abstraction. I am planning to read "Growing Object-Oriented Software Guided by Tests" book. I hope that will help me to identify actual abstraction in my design/code.

Answer 1

Mark Seemann makes a good point, but it's important to not take these things as a universal law, and understand them with more nuance. Your GetMenuList method might be too simple for this to apply, but if not, the kind of abstraction the blog is talking about would be based more on the part you omitted (..........), than on the GoogleSheetsService itself. As in, it would be an interface with methods that express the needs (or major steps) of the GetMenuList function, rather than being a generic interface for accessing/modifying sheet data.

I'm not necessarily recommending the following because I don't know exactly what GetMenuList is doing, but for illustrative purposes, suppose the return value was not a simple list, and that the menu logic was a bit more involved (so that there are tangible steps other than just "get the data"), and also that you have to support different versions of the sheet, that differ in how the data is stored. Then, you wouldn't want to hard-code this logic within the function itself - instead, you'd come up with a more abstract interface with methods like (and I'm just making this up)

GetStandardMenuItems(); 
GetCustomerSpecificItems(customerId); 
GetMenuLayout();
// etc. 

- or something along those lines. GetMenuList would use these internally to express its logic.

In other words, it's a "role interface" consisting of higher-level operations that can be considered common across all the different implementations (what they are exactly will depend on the details of the domain). This interface is an abstraction in the sense that something like GetMenuLayout(...) is more directly expressing what you're trying to achieve (within the context of the caller), whereas GetDataFromSheet(...) is more about the details of accessing generic data (that you ultimately use to achieve it), so this higher level interface is "abstracting away" the specifics of the data-access service. This is, really, dependency inversion applied (it's this sort of abstraction that actually makes it work).

You'd write the logic (and tests) in terms of those higher-level methods, and you'd then delegate reading the data to different implementations of that interface, each of witch encapsulates the knowledge of which cells to access for each particular version. The mocks used within the tests for GetMenuList wouldn't need to emulate the details of an actual sheet, they'd just set up the functions to return the "canned" results needed for a particular test (no more, no less). The kinds of tests you'd write would not be about reading the data, but about how the received information about menu items, customer-specific menu items, and the menu layout is being combined together (or whatever this imaginary version of GetMenuList is supposed to be achieving - you'd test the rules associated with that process).

You'd test the data-accessing dependencies (interface implementations) separately, for things like: are the right cells being read when a method is called (maybe by checking the parameters passed to the underlying service/library that ultimately reads the data). So these tests aren't a repeat of the previous ones, but cover a different set of behaviors. You wouldn't necessarily need to extract a "header interface" out of GoogleSheetsService, but you might need to adjust the design of that general part of the application (by modifying the APIs of your service classes, and/or introducing helper classes) to make this kind of testing easier - if the team feels doing so is worth it in order to get more robust tests. If not, maybe you just use premade test sheets and call it a day (you may re-evaluate that decision at some later point in time, if the need arises).

These unit tests check if the individual components are doing what they are supposed to be doing, and it's important to be clear about what the responsibilities of each component are, so that you know precisely what to test for, and also limit the scope of the tests. Integration tests would give you further confidence that they also work together.

Sometimes, design goals are in conflict with the typical way of using the library, or there are performance considerations, or the added complexity doesn't pay off - so there are tradeoffs involved (but hopefully you can limit the impact of these to the periphery of the application).

That said, in your case, maybe the intermediary level of abstraction of the kind described above is not needed, and maybe you could shift the perspective to also include the calling code, with GetMenuList now being viewed as a suitable abstraction hiding the details of the sheet-reading service from the client code. Again, I don't know what the calling code looks like, or if there is a need for this, but one thing you could consider instead is extracting the IMenuData interface, with GoogleSheetMenuData being one implementation of it, while tweaking the callers so that they aren't explicitly mentioning implementation-specific aspects like "mastersheetid" (although a string "key" is pretty general, so might not be a problem). This then potentially enables you to have your higher level (calling) code support different sources of data with minimal modification.

If going down that route, don't make it too complicated and don't make too many early assumptions, because coming up with an abstraction that turns out to be unsuitable down the line could potentially make things much worse (this is why we have the "rule of three" - it's supposed to help us get a better idea of the patterns and commonalities before we decide what to do). On the other hand, it's also important not to think of the design decisions made early as set in stone, and react in a timely manner (which is often sooner than you'd think), before the codebase becomes a nightmare to work with.

Answer 2

How to abstract a dependency of subject under test?

The short answer here is to use an interface and set your dependency as the interface type instead of the concrete class that implements said interface.

The reason is that an interface innately allows you to mock its implementation, because you can just create a different implementation of the same interface, one which mocks the behavior however you want it to. You can also look at mock libraries like NSubstitute to dynamically create mocks instead of doing it formally via a mocked class definition.

Your implementation of MenuData is atypical. Generally, the dependcies go via the constructor, the ID goes via the method. You're doing it the other way around. It's unclear to me whether this is an intentional design decision or an oversight.

---

I'm keeping the above direct answer short, because I want to instead pivot into discussing the blog post you linked and why I think it's a bad source of advice for you to rely on.

The conflict in your question (i.e. the thing that's driving you to ask it here) seems to be based on the linked blog post and your struggle with implementing what it preaches, so I'm going to focus on the content in the blog post with which I disagree, to justify your struggles and highlight that the advice being given is sometimes misguided or lacking in nuance.

Personal opinion disclaimer: I really dislike the blog post. Not because it's totally wrong (it makes a few good points) but it's written with a tone of authority, engages in blanket statements, selectively curates its content to only include things that agree with the author's intent, and makes several rookie mistakes in the process which completely undercut its authoritative tone.

The rest of this answer will be a response to why the advice given in the blog post is inaccurate, misguided, dead wrong, or in some cases (in my opinion) intentionally doctored to support the content of the blog post.

"Program to an interface, not an implementation"

An interface is just a language construct.

That blog post falls flat on its face right out of the gates, as it conflates "program to an interface" to mean "program to an interface" (i.e. the language construct). This is not the case, and undermines the entire point that the blog then goes on to make.

For the sake of completeness, in "program to an interface", "interface" refers to any public contract. The interface construct is one of those, but it's not the only one. Any class' public content (methods, properties, even fields) also constitutes an interface.

When you subtract that undermined argument, what's left is a claim that using an interface alone does not sufficiently provide an abstraction. This is not incorrect, but it is not particularly insightful. Anything can be done badly and/or to a point of defeating its own purpose.

Examples of [thing] being done badly should not be used as argument why [thing] is bad; they're argument of why whoever did [thing] badly did a bad job. Spoiler alert: we will come back to this point several times.

However, as Uncle Bob points out, even an interface as simple as this seemingly innocuous rectangle ‘abstraction' contains potential dangers:

The Rectangle problem suffers from implicit expectations about how it should operate. It hinges on an assumption that changing one property should never ever affect another, but this is simply not a hard rule in software development.

If you assume that all properties are independent of one another and cannot possibly impact one another, then you're thinking about DTOs, not logical models, which is an oddly (and selectively) restrictive interpretation of what your class (and its abstracted interface) could represent.
Yeah, interfaces don't make a lot of sense on DTOs, because they're dumb data containers that don't have any real logic to them, and LSP is a logic violation, not one of data storage structure.

But even if we humor the author that their implicit inference based on the interface's definition is inevitably the only possible interpretation (which it isn't), then the designer of the interface should be blamed for writing a bad interface. It's nonsensical to use this to argue against the very concept of interface itself.

The problem stems from the fact that the operations have side effects. Invoking one operation changes the state of a seemingly unrelated piece of data. The more members we have, the greater the risk is

Notice here the introduction of "risk" as a description of this scenario. The author is inherently implying that side effects are universally bad.

If you're that averse to side effects as a blanket measure (which is fine, it's definitely one way to approach your design), then you should be using pure functions and immutable objects; at which point the Rectangle example is rendered moot, as the entire objection hinges on changing values post-initialization.

So, pick a lane: either side effects are not all bad, or (if you think they are all bad) you should never have designed an interface that enforces mutability in its implementors. In either case, the proposed issue with the Rectangle problem is not an accurate example.

Header interfaces [..] interfaces mechanically extracted from all members of a concrete class are poor abstractions.

Again we strike on the same issue where doing [thing] badly does not argue against [thing], it argues against the developer who badly did [thing].

For all the harping on about Liskov (the L in SOLID) in the previous chapter, the blog suddenly falls quiet on the Interface Segregation Principle (the I in SOLID), even though it targets this exact problem that the blog post is pointing out.

However, ISP inherently also justifies the use of interfaces (as long as they're propertly segregated), which is why I'm suspecting that the author very much avoided including ISP in the blog post.

Shallow interfaces

This section is egregiously badly written. Not grammatically, but in terms of the technical opinion that it tries to get across. It is so bad that it would be best skipped in its entirety when reading the blog post.

But let's dig in.

[..] because it doesn't recursively extract interfaces from the concrete types exposed by the extracted members.

First of all, that's not a complaint against the language, let alone its interface construct; that's a complaint about an IDE automation that you think could have worked a bit differently. This is completely unrelated to the actual topic at hand. Judging a language (let alone a language-agnostic OOP paradigm) by one IDE's implementation of a code generation utility is patently moronic, and is one of the main reasons why I personally believe that the author of this blog post did not attempt to make a good faith argument.

Secondly, and more importantly, the vast majority of concrete types that get referenced in public method signatures and public properties are data containers (i.e. DTOs), where extracting an interface is pointless as it contains no real data.

The assertion that all concrete types should be extracted at the same time is fraught with problems. From a UI perspective, it could cause a massive chain of window popups to extract an entire application's dependency graph. At the same time, a large majority of its windows would be entirely moot as they are DTOs, and the author's underlying argument here is that language paradigms are only as good as the IDE automations that get put on top of them.

At first glance this may look useful, but it isn't. Even though it's an interface, it's still tightly coupled to a specific object context. Not only does ObjectSet reference the Entity Framework, but the Posting class is defined by a very specific, auto-generated Entity context.

Neither of which is the task of the interface-extracting code generation tool. If the author is claiming that the direct consumer of PostingContext should not have access to EF-dependent types, then PostingContext (the class) should never have been built in a way to expose these EF types in the first place. The only reason the tool exposes these types in the interface is because the class had already exposed them to begin with, it simply followed suit with the class whose public interface it was explicitly told to copy.

This seems like a very cheap shot to seemingly support the blog post's narrative. The interface extraction tool only derives an interface from how the class was designed. The author here designed a class' public interface badly, then set the extraction tool to mirror this in the interface that it generates, and then the author turns around and blames the tool for producing a flawed interface.

If that's not the pot calling the kettle black, I don't know what is.

Leaky Abstractions - Another way we can create problems for ourselves is when our interfaces leak implementation details.

Leaky interfaces are a design problem. Even without interfaces, your classes would be leaking the same way. This is wholly unrelated to the interface construct.

Exposing a ConnectionString property strongly indicates that the repository is implemented on top of a database

Yeah, again this is just a bad design, not an argument against the interface construct.

More amusingly, the author seems to have a very restricted understanding of what connectionstring are actually used for. Message brokers/queues, caching services, email clients, IoT devices, microservice frameworks (e.g. Eureka), file storage (e.g. FTP), elastic search, and even the blockchain all make use of connectionstrings.

You shouldn't be exposing connectionstrings regardless of which technology uses them, so I'm questioning the author's argument that this design is somehow a good idea in the first place before we even think of introducing an interface or not.

Conclusion - In short, using interfaces in no way guarantees that we operate with appropriate abstractions. Thus, the proliferation of interfaces that typically follow from TDD or use of DI may not be the pure goodness we tend to believe.

"Conclusion - In short, using a seatbelt in no way guarantees that you will survive every single car crash. Thus, the proliferation of seatbelts that typically follow from legislation or ANCAP recommendations may not be the lifesaver we tend to believe"

I think the flawed logic is obvious here. The author thinks that a counterexample completely dismantles the core concept.