Hacker News new | ask | show | jobs
by vikinghckr 1262 days ago
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.
3 comments

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.