Hacker News Reader

Hi, I really enjoyed the piece! How much to test at each layer of abstraction has always been something that's bugged me. This article provides a nice strategy.

I still have questions about a few scenarios though. Let's say, I have two classes like this:

  class KeyValueStore {
    int64 Get(Key key);
  };

  class CachingKeyValueStore {
    CachingKeyValueStore(const KeyValueStore& wrapped);

    int64 Get(Key key);
  }

Here, the `CachingKeyValueStore` class is a wrapper around the `KeyValueStore` class, and its purpose is to simply maintain an in-memory cache (instead of RPC calls or Database calls). How would you unit test the `CachingKeyValueStore` class? So far, using Mocks have been my only strategy, because behavior-wise, both classes have the exact same outputs for the same inputs.

Thanks! I'm glad you enjoyed it.

The behavioral difference between Caching KeyValue Store and KeyValueStore is that it doesn't fetch from the underlying data source when called a second time. So that's what I would test. Given the signature you provided, I would either instrument KeyValueStore so I could tell if it had been run twice, or (if KeyValueStore has multiple implementations) I might create a ExampleKeyValueStore in my tests that had that instrumentation.

I solved this problem a bit differently in my code, though. Instead of making a CachingKeyValueStore, I made a MemoryCache. It has a get() method that takes a key and a function. The first time it's called, it runs the function; the second time, it returns the stored result. Here's one of my actual tests for that code (JavaScript):

  it("caches result of function, so it's only run once", async function() {
    const cache = createIndependentCache();

    let runCount = 0;
    const getFn = () => runCount++;

    await cache.getAsync(NULL_LOG, "key", getFn);
    await cache.getAsync(NULL_LOG, "key", getFn);

    assert.equal(runCount, 1);
  });

And in case you’re curious, here’s the implementation, excluding a bunch of instrumentation and logging:

  async getAsync(log, key, getFnAsync) {
    if (this._cache[key] === undefined) {
      this._cache[key] = getFnAsync();
    }

    try {
      return await this._cache[key];
    }
    catch (err) {
      delete this._cache[key];   // don't cache errors
      throw err;
    }
  }

Thanks! I really like the idea of the instrumented ExampleKeyValueStore class which can be defined like this:

  class ExampleKeyValueStore {
    ExampleKeyValueStore(KeyValueStore wrapped);
    int hit_count();
  };

That means I can use the real KeyValueStore as a dependency while still testing the caching behavior. Cool!

You probably should have some additional tests around the CachingKeyValueStore related to eviction and size. Maybe this doesn't matter very often, but you should at least test the behavior of your Caching store if when the cache size is only, e.g. 1 item.

You can either do this by passing in a mock KVStore, or by passing in a normal/fake KV store in which you can update the underlying data. So for example:

    data = ...
    kv = new KVStore(data)
    cache = new CachingKVStore(kv, cache_size=1)
    v = cache.get(k1)
    data.update(k1, new_value)
    v2 = cache.get(k1)
    assert v2 == v1  // Cached!
    v3 = cache.get(k2)  // evict k1
    v4 = cache.get(k1)
    assert v4 != v1
    assert v4 == new_value  // Gets the new value

Otherwise, yeah for some cases like this you can just inherit from the KVStore test class and run all the same tests with a slightly different set up method to use a caching store instead.

Thanks! Injecting the data itself is an interesting approach!

I would use a mock for the wrapped instance that generates a strong random value on each call to “Get”.

Then I would do some basic unit tests and maybe property based testing given that the only way for the same value to appear on subsequent “Get” calls is for caching to have occurred.

> I could test that with broad tests, but doing that in a large system means that you end up with a combinatorial explosion of tests, or (more likely) hidden gaps in your tests.

Extremely well put. This maps exactly to my experience when moving from unit tests to the layer above. Test are taught exactly like “draw the rest of the fucking owl” meme, and I’ve never seen them actually hold up to the nice and tidy “hello word” tests that you see in demos. You’re actually trying to tackle the reality of gritty and ugly software systems, and it’s very refreshing to see, because it may just work. It’s gonna take a while to see if it holds up, but so far I am really excited.

For logic heavy parts of the system then yes, narrow tests are very useful. Often these sorts of components are amenable to property based testing which definitely becomes harder the broader the surface. I have generally been lucky that these bits of code are either rare, or they become infrastructure dependencies quite quickly in which case you’re broadly testing just categories of responses, not all edge cases.

But I still think a general rule to live by is to maximise the ratio of business value covered by tests to the amount of program text referenced by tests. Too small a ratio and you’re gaining little confidence but exposing yourself to constant toil just to keep up with changing implementations, even when observable system behaviour doesn’t change much.

I’m unsure if top level sociable tests get me where I want to be. It seems like they stop one layer down, so I’m still concerned I don’t capture actual end to end use cases anywhere in the codebase. I’m not personally comfortable with that (from a testing point of view but also because I’m tired of working on codebases that don’t clearly reveal what they do and why), but that outlook’s been formed by my own particular failures and shortcomings over the years so it might be quite subjective.