[patch] hashcache fixes

[Robert Collins]

On Mon, 2006-07-10 at 10:10 +1000, Martin Pool wrote:
> On  9 Jul 2006, Robert Collins <robertc at robertcollins.net> wrote:


> > > +        if not hasattr(os, 'mkfifo'):
> > > +            raise TestSkipped('filesystem fifos not supported on this
> > > system')
> > 
> > This seems to be a case for just returning to me - its not something
> > that the person running the testcase can correct. (Which is what we
> > defined TestSkipped to mean).
> 
> No, I really don't think so.  Silently not running tests is really no
> good - what if I misspelled 'mkfifo' and so it always returned?  When
> Aaron's refinement of TestSkipped lands we can raise an error indicating
> 'we can't test this on your OS'. 
> 
> There's a useful semantic difference between "this passed" and "this
> couldn't be tested".

Indeed. We specced this out in http://bazaar-vcs.org/BzrExtendTestSuite.
I haven't been tracking the list well this last few weeks, as I've been
in $hectic mode. What thread should I read to see about Aarons
refinement of TestSkipped? (Is it just an implementation of the
BzrExtendTestSuite, or something different ?)

> We were talking the other day about trying to make sure that there are
> zero skipped tests on fully-fledged developer machines, and if there are
> any skips that indicates something is wrong.  The flip side is that we
> should never return rather than skipping.

If we drop support for format 0.4 branches, and refine our interface
defitions, then this is a doable target IMO.

> > +    def test_hashcache_hit_old_file(self):
> > +        """An old file gives a cache hit"""
> > +        return ### this takes too long to run properly; skipped
> > 
> > This seems to be something to raise TestSkipped on - it is something
> > that can be changed to run.
> 
> OK, I can imagine skipping that as a slow test or something.
> 
> > That said, heres what I'd like to discuss - some of the tests for the
> > hash cache seem fragile, because we're depending on the external system
> > to line up with our expectations to trigger failures. 
> 
> I think the new tests will not be fragile, in the sense that they should
> not falsely-fail if they're run on a very slow or a very fast machine.
> (The old ones could.)
> 
> > I think it would
> > make sense - and as we are working on this right now - to to simulate
> > the external conditions that can cause grief, and run the hashcache
> > tests against such a simulated system, where we can say 'stating this
> > file will return a mtime with grantularity of 2 seconds' and 'the system
> > time is now 1 second later' without needing *either* a FS with 2 second
> > granularity, nor needing to wait for the system clock to actually move
> > forward 2 seconds.
> 
> Yes, when the real world is too hard to test against using a simulator
> is a good idea.
> 
> However any simulation is only as good as your understanding of the
> situation, and if the bug is caused by misunderstanding how the system
> behaves it won't catch it.  I would not have imagined the time could
> apparently step backwards as it did here.
> 
> Of course once you realize that this can happen you can update the
> simulation.
> 
> But on the other hand tracking down the bug by writing a test against
> the real world, and then leaving (a cleaned up version of) that test in
> the suite is good.

I can best describe my feelings about this with an analogy - if we had a
bug deep in transport, that was exposed by the command line, we would
expect to add a test of transport, and possibly a test of the
commandline that checks the right sequence of calls are made by the
specific command, using an instrumented, or simulated transport
implementation. What we would rarely do is add a new integration test.

> For now this bug is closed and in any case we expect to rework this code
> in the future (folding it in with the working inventory).  Simulating
> the filesystem might be good in the future.

Well, there will be something folded into the dirstate, but this code
will stay around to support current-format trees in all probability.

> So for now I propose to land this, treating simulation of the filesystem
> as just a todo.

As long as the test you disabled via 'return' is enabled, I'm ok with
this. What I was concerned about was having the key test disabled, with
no sign its disabled, 

-Rob
-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060710/2cdd5443/attachment.pgp