[patch] hashcache fixes

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> OK, here is an additional patch that
> 
>  - moves the functions that depend on the OS state into HashCache
>  - removes dead 'FixThisError' (??) from test_hashcache
>  - adds new FakeHashCache, which gives the test suite control of 
>    how the files and clock appear to the hashcache
>  - new tests using this which check three key behaviours:
>    - new files are not inserted in the cache
>    - old files are inserted in the cache and can hit
>    - if a file is modified within a short time window, we don't get
>      an incorrect cached result
> 
> Obviously we could make the fake cache much more sophisiticated but I
> think this fixes the immediate problem and gives a foundation for future
> work.  What do you think?
> 
> There are now no skipped or stubbed-out tests here and none that may
> pass or fail depending on environment variances.
> 

+1 on concept, but some of the code seems to be missing for the tests.

...

...

> +    def _cutoff_time(self):
> +        return self._clock - 2

Do we want -2 or -3 here?

> +
> +    def pretend_to_sleep(self, secs):
> +        self._clock += secs
> +
> +    
> +class TestHashCacheFakeFilesystem(TestCaseInTempDir):
> +    """Tests the hashcache using a simulated OS.
> +    """
> +
> +    def make_hashcache(self):
> +        return FakeHashCache()
> +
> +    def test_hashcache_miss_new_file(self):
> +        """A new file gives the right sha1 but misses"""
> +        self.assertEquals(hc.miss_count, 1)
> +        self.assertEquals(hc.hit_count, 0)
> +        # if we try again it's still too new; 
> +        self.assertEquals(hc.get_sha1('foo'), sha1('hello'))
> +        self.assertEquals(hc.miss_count, 2)
> +        self.assertEquals(hc.hit_count, 0)

^- don't you need a 'hc = self.make_hashcashe()' here? I don't know how
this was passing.


> +
> +    def test_hashcache_old_file(self):
> +        """An old file gives the right sha1 and hits"""
> +        hc = self.make_hashcache()
> +        hc.put_file('foo', 'hello')
> +        hc.pretend_to_sleep(20)
> +        # file is new; should get the correct hash but miss
> +        self.assertEquals(hc.get_sha1('foo'), sha1('hello'))
> +        self.assertEquals(hc.miss_count, 1)
> +        self.assertEquals(hc.hit_count, 0)
> +        # and can now be hit
> +        self.assertEquals(hc.get_sha1('foo'), sha1('hello'))
> +        self.assertEquals(hc.miss_count, 1)
> +        self.assertEquals(hc.hit_count, 1)
> +        hc.pretend_to_sleep(3)
> +        # and again
> +        self.assertEquals(hc.get_sha1('foo'), sha1('hello'))
> +        self.assertEquals(hc.miss_count, 1)
> +        self.assertEquals(hc.hit_count, 2)
> +
> +    def test_hashcache_invalidates(self):
> +        hc = self.make_hashcache()
> +        hc.put_file('foo', 'hello')
> +        hc.pretend_to_sleep(20)
> +        hc.get_sha1('foo')
> +        hc.put_file('foo', 'h1llo')
> +        self.assertEquals(hc.get_sha1('foo'), sha1('h1llo'))

^-- Shouldn't this also check the hit/miss values?
Also, we might want to put in another test that does another sleep


Generally positive, but why isn't the test_hashcache_miss_new_file()
failing?

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEs+mxJdeBCYSNAAMRAt9eAKConO3ot+B0w3WVLk8XbMocpv10RwCbB/9i
GS50bJHTcIqWzhiXY5cMNq4=
=EVIK
-----END PGP SIGNATURE-----