[darcs-devel] shrinkPatchInfo goes into infinite loop

Ganesh Sittampalam ganesh at earth.li
Sun Mar 1 21:15:50 UTC 2020


Hi,

On 28/02/2020 21:19, Ben Franksen wrote:

> I'll send my patch. If you apply it and roll back the fixes I made to
> metadataDecodingTest, you tests should loop (use darcs-test -t
> Darcs.Patch.Info).
> 
> [...] I guess the problem is related to the way
> you define shrink via <|> for the member of Patchinfo.

That approach is just meant to shrink each element of the PatchInfo in
turn. I don't think it's a problem.

I've spent a while playing with this and I am partially convinced that
it's your change that triggered the loop.

I've just sent some new draft patches on top of yours in patch2006. If
you just take these ones:

  * introduce empty
  * tests: bugfix for shrinking log lines
  * WIP: introduce shrinking tests

You'll see that the new tests I added to check shrinking pass.

If you then take the rest, they start failing. My suspicion is that your
change to the shrinking of names:

> -      sn <- f1 (piName pi)
> +      sn <- f1 (justName pi)

along with the other underlying changes, has made it possible for an
empty name to be shrunk to an empty name. But I'm not 100% certain so
I'm happy to keep looking into it if you still think it's a problem with
the existing shrinking.

The general work on shrinking tests is very much provisional. It could
do with a bit more documentation, and I'm a bit nervous about the extra
code to maintain.

The basic idea is to firstly check that all shrinks are valid (as the
PatchInfo ones weren't previously) and secondly that they all make the
value smaller.

This was surprisingly tricky to get right for PatchInfo, partly because
of all the hidden invariants/representation assumptions embedded in the
type. Your refactoring is of course moving in the right direction to
normalise the representation.

The Char instance in particular was a pain, I had to read the source for
shrink on Char to understand how to write it. And if that changes it'll
be a maintenance headache.

Ganesh


More information about the darcs-devel mailing list