[patch] hashcache fixes

[Andrew Bennetts]

On Sun, Jul 09, 2006 at 10:32:40PM +1000, Martin Pool wrote:
[...]
> 
> > Would it be better to directly update
> > the mtime of those files in the tests instead?
> 
> No, because the code specifically checks the ctime to guard against
> files which are new but whose mtime has been rewound. :)

Heh.  :)

I assumed you could also set ctime directly, but digging around a bit I appear
to be wrong about that.

> But responding to this did make me think of something else: we can get a
> bit closer by looking at an older file outside of the test directory,
> such as say the oldest file in the bzr source directory.  We only need
> read access to prove the cache can hit.  That should be pretty
> straightforward.
> 
> But how to automatically test the case of starting with an old file,
> caching it, then updating it, without actually delaying?

Without be able to completely control the relevant metadata for a file, I guess
we can't.

Looking at the problem from a strict testability point of view, this suggests
the code isn't factored well enough.  What if the _fingerprint function was a
method of HashCache objects instead, so test cases can override it to return
modified results -- e.g.:

    class InstrumentedHashCache(HashCache):
    
        def _fingerprint(self, abspath):
            # always return a ctime at least 1 day in the past
            fp = list(HashCache._fingerprint(self, abspath))
            fp[FP_CTIME_COLUMN] -= 86400
            return tuple(fp)

Or whatever logic the test wants (e.g. you could conditionally change behaviour
depending on the filename).

There may be practical reasons not to do this, though -- there may be a
measurable performance impact to using a method rather than a global function.
I guess the tests could always replace the global function temporarily too, but
that's nastier (although a more localised form of nasty).

-Andrew.