Okay, here is the long promised improved test case generator for
RepoPatches, together with a few other improvements to the test harness.

10 patches for repository http://darcs.net/screened:

patch b5f82f0bd3b4a6c02d4f5ca39a86b1ee876da43a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 21:47:26 CEST 2019
  * harness: move nontrivialX conditions to D.T.P.A.Generic and remove dead code

patch 94f6ecd73fa1a64c7d6713e47c74e8e5a0f2a3b7
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 15:41:36 CEST 2019
  * harness: remove a dirty hack from patch tree generators
  
  This re-adds a slightly modified version of sizeTree which we use to
  calculate the number of pairs in a flattened Tree.

patch 075808841224ddd930f041c723ae21ede6bb723e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  7 10:16:26 CEST 2019
  * harness: make sure once and for all that generated Trees have enough patches

patch 846bc17be395d01b82257f66aedfadf187caafb5
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  7 10:15:44 CEST 2019
  * harness: minor cleanups in D.T.P.A.Generic

patch 47cc5a8ba40f7b43e8611678d8f5949f578b7ac4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 28 10:43:09 CEST 2019
  * harness: make encoding roundtrip test faster

patch d3cf9a7597b52a79209afe80ee12d9778637ce84
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  7 08:43:00 CEST 2019
  * harness: treat empty hunks specially in checkPatch
  
  See the comment in the code. I wonder why this hasn't come up earlier.

patch b0a689417ade345d2f7b60e7d8969439a96d5710
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  7 10:00:51 CEST 2019
  * harness: remove code we no longer need

patch 5f2996bf7f298c8445740f9fe847fc046dbfd17c
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  7 10:00:08 CEST 2019
  * harness: never generate empty hunks

patch 08d6977bb518bca5024ecad7e87fc6fd29055d79
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 24 08:30:22 CEST 2019
  * improved test case generator for RepoPatches
  
  We previously generated RepoPatches by merging prims from a Tree. While this
  generates conflictors, it never generates sequences where a patch depends on
  a conflictor. The new generator (which can only be used for patch types that
  have a Merge instance i.e. not prims) directly generates an RL of patches,
  making sure we cover all possible cases.

patch 39f81cf1b5ebc727677b74fe97e20578af520f2f
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Jul  4 22:22:53 CEST 2019
  * harness: limit number of flattenings in propConsistentTreeFlattenings
  
  The number of flattenings grows exponentially in the worst case, so this
  helps to keep the test reasonably efficient even if we crank up the number
  of QC tests.

Attachments

• harness_-move-nontrivialx-conditions-to-d_t_p_a_generic-and-remove-dead-code.dpatch
• patch-preview.txt
• unnamed

> patch b5f82f0bd3b4a6c02d4f5ca39a86b1ee876da43a
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Jun 21 21:47:26 CEST 2019
>   * harness: move nontrivialX conditions to D.T.P.A.Generic and 
>     remove dead code

OK

 
> patch 94f6ecd73fa1a64c7d6713e47c74e8e5a0f2a3b7
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Jun 21 15:41:36 CEST 2019
>   * harness: remove a dirty hack from patch tree generators
>   
>   This re-adds a slightly modified version of sizeTree which we 
>   use to
>   calculate the number of pairs in a flattened Tree.

OK

> patch 075808841224ddd930f041c723ae21ede6bb723e
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sun Jul  7 10:16:26 CEST 2019
>   * harness: make sure once and for all that generated Trees
>     have enough patches

OK

> patch 846bc17be395d01b82257f66aedfadf187caafb5
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sun Jul  7 10:15:44 CEST 2019
>   * harness: minor cleanups in D.T.P.A.Generic
OK 
> patch 47cc5a8ba40f7b43e8611678d8f5949f578b7ac4
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Fri Jun 28 10:43:09 CEST 2019
>   * harness: make encoding roundtrip test faster
OK

 
> patch d3cf9a7597b52a79209afe80ee12d9778637ce84
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sun Jul  7 08:43:00 CEST 2019
>   * harness: treat empty hunks specially in checkPatch
>   
>   See the comment in the code. I wonder why this hasn't come
>   up earlier.

OK. 

> patch b0a689417ade345d2f7b60e7d8969439a96d5710
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sun Jul  7 10:00:51 CEST 2019
>   * harness: remove code we no longer need

OK

> patch 5f2996bf7f298c8445740f9fe847fc046dbfd17c
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Sun Jul  7 10:00:08 CEST 2019
>   * harness: never generate empty hunks

OK

> patch 08d6977bb518bca5024ecad7e87fc6fd29055d79
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Mon Jun 24 08:30:22 CEST 2019
>   * improved test case generator for RepoPatches
>   
>   We previously generated RepoPatches by merging prims from a 
>   Tree. While this
>   generates conflictors, it never generates sequences where a
>   patch depends on
>   a conflictor. The new generator (which can only be used for
>   patch types that
>   have a Merge instance i.e. not prims) directly generates an RL
>   of patches,
>   making sure we cover all possible cases.

Nice, thanks.

> patch 39f81cf1b5ebc727677b74fe97e20578af520f2f
> Author: Ben Franksen <ben.franksen@online.de>
> Date:   Thu Jul  4 22:22:53 CEST 2019
>   * harness: limit number of flattenings in
>     propConsistentTreeFlattenings
>   
>   The number of flattenings grows exponentially in the worst case, 
>   so this
>   helps to keep the test reasonably efficient even if we crank up 
>   the number of QC tests.

OK

>   * improved test case generator for RepoPatches

Perhaps unsurprisingly, I see random failures in the V2 QuickCheck 
tests with this patch included. I'm not sure what to do about that.
Should we just disable the properties for V2 that can fail, or can
we characterise the known bugs in V2 well enough to just exclude 
test cases that trigger them?

>>   * improved test case generator for RepoPatches
> 
> Perhaps unsurprisingly, I see random failures in the V2 QuickCheck 
> tests with this patch included. I'm not sure what to do about that.
> Should we just disable the properties for V2 that can fail, or can
> we characterise the known bugs in V2 well enough to just exclude 
> test cases that trigger them?

It was surprising to me, at least, as I never stumbled over any of these
failures in real life. But I found that others did, see e.g.
https://hub.darcs.net/simon/darcsden/issue/183. I really tried to
understand the code involved but failed miserably. In fact, I have not
the slightest idea how or why this code is supposed to work.

Characterizing the known bugs precisely would amount to getting an idea
why it goes wrong in the first place, so I guess the chances are slim
here. The only thing I know is that they weren't triggered before this
change. So instead of disabling the tests for RepoPatchV2, we could
revert them back to using the simpler test case generator. This will
complicate the test code a bit, but hopefully not too much.

> So instead of disabling the tests for RepoPatchV2, we could
> revert them back to using the simpler test case generator. This 
> will complicate the test code a bit, but hopefully not too much.

That makes sense - just to check, will you do that? I could also
have a look if you're busy.

>> So instead of disabling the tests for RepoPatchV2, we could
>> revert them back to using the simpler test case generator. This 
>> will complicate the test code a bit, but hopefully not too much.
> 
> That makes sense - just to check, will you do that? I could also
> have a look if you're busy.

Don't bother, I have made a patch a few days ago. Will send.

Following up on review. The last patch is what I wanted to send originally,
but the dependencies are actually all nice improvements and thus included.

4 patches for repository http://darcs.net/screened:

patch 13abf76b8f18bc3b7c4e2f32b6043865178a00bc
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Jul 16 19:41:32 CEST 2019
  * harness: simplify constraints in D.T.Patch

patch d97bf8e299efcf9078ddc84c7255c46121aa8d40
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Jul 17 17:20:18 CEST 2019
  * harness: add propConsistentReorderings and test it
  
  This property is similar to propConsistentTreeFlattenings but takes an RL
  RepoPatch and a start state as input. This allows us to test it more
  thoroughly by using the new test case generator for RL RepoPatch.

patch cc41cb35f9e5dab29783b9c7a1f0ef5df29a19f1
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Jul 17 17:21:57 CEST 2019
  * harness: simplify type signatures and superclass constraints

patch 4882dda3251975b08fc8fa003c08ca3c7b523520
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Aug  1 23:17:51 CEST 2019
  * use the old test case generator for most RepoPatchV2 properties
  
  With the new test case generator many properties fail for RepoPatchV2, so we
  go back to the simpler generator that does not record patches after
  conflicts. This required re-adding merge_properties and also adds
  triple_properties (for permutivity). While we're at it, also test the
  triple_properties for prim patches.

Attachments

• harness_-simplify-constraints-in-d_t_patch.dpatch
• patch-preview.txt
• unnamed

>   * harness: simplify constraints in D.T.Patch

OK

>   * harness: add propConsistentReorderings and test it
   

OK (I think some of the cleanups from the next patch ended up here 
too. No big deal though.)

>   * harness: simplify type signatures and superclass constraints

OK

>   * use the old test case generator for most RepoPatchV2 
>     properties
>   
>   With the new test case generator many properties fail for
>   RepoPatchV2, so we
>   go back to the simpler generator that does not record patches 
>   after
>   conflicts. This required re-adding merge_properties and also 
>   adds
>   triple_properties (for permutivity). While we're at it, also 
>   test the
>   triple_properties for prim patches.

OK, so now we have 'difficultRepoPatchProperties' for V3, and 
'merge/pair/patch/triple_properties' for V2 which should be most of 
the same set of tests but with different generators.

A future improvement would be to find a way to just parametrize over 
the generators so we can reuse the same collection for both V2 and 
V3 with a separate extra V3-only list if needed. But this is fine 
for now.