On Wed, 23 Sep 2009, Eric Kow wrote:
Still plodding along for this review.
On Sat, Sep 19, 2009 at 11:54:08 +0100, Ganesh Sittampalam wrote:
Mon Aug 31 00:46:47 BST 2009 Ganesh Sittampalam <[email protected]>
* add some QC properties that demonstrate the problem with canonizeFL
Pushing this one... albeit with a slipshod review :-(
Apologies: it was a slipshod submission too. I forgot that it depends on
my patch to add witnesses to Darcs.Diff. That patch probably won't go in
in its current form because it conflicts with darcs-hs, so I'm rolling
this back for now.
add some QC properties that demonstrate the problem with canonizeFL
-------------------------------------------------------------------
Silly Eric...
data RepoModel C(x)
= RepoModel {
rmFileName :: !FileName,
rmFileContents :: [B.ByteString]
} deriving (Eq)
My brain wouldn't stop reading that 'rm' as 'remove' instead of
'repoModel'. I kept wondering why we were trying to track these
removes.
I actually have that problem with it too despite having written it. It
should be fixed :-)
Does this mean in general that our unit tests operate only on one-file
repos?
The unit tests in that module have that property, but they are primarily
intended to test the basics of patch theory so that's fine. I was sort of
piggy-backing on them with this test because I knew that the pieces I
needed for it were there. However eventually they should probably be
generalised.
+canonicalDiff :: RepoModel C(x) -> RepoModel C(y) -> FL Prim C(x y)
+canonicalDiff rm1 rm2 = diff [IgnoreTimes] (const TextFile) (repoModelToSlurpy
rm1) (repoModelToSlurpy rm2)
This will be better when Neil's CmdArgs work lands
I don't think this aspect of the option handling is a big deal.
+instance Arbitrary (Sealed RepoModel) where
+ arbitrary = do wes <- arbitraryState initRepoModel
+ let _ :: Sealed (FL Prim C(InitRepoModel)) = mapSeal wesPatch
wes
+ return (mapSeal wesState wes)
Out of curiosity, why not just let foo :: and return foo?
Errm. Dunno :-) The let _ .. would have been added late to get past the
type-checker, so I suspect I just didn't notice what I was doing.
I don't really know much about QC so I'm learning as I go. Shrink just
returns a list a possible shrinks - which I take to mean simpler
candidate values. I'm guessing that the use for this would be to see if
you can get the same failure out of a shrink, in which case you should
just use that to avoid reporting a whole lot of gunk. But that's just a
guess.
Correct.
+shrinkList [] = []
+shrinkList (x:xs) = xs:map (x:) (shrinkList xs)
I think the idea here is that each candidate shrink is just the result of
getting rid of some lines
Isn't this just some common list idiom like powerset? (Which is not to say
that we should necessarily use any of the fancy liftM tricks, but maybe just
call it what it is).
It's not powerset, because it only removes one element at a time. I'm
surprised it's not in the QC library somewhere, but I couldn't spot it so
just wrote it locally.
Now to the meaty bits. Basically we want to check if canonicalDiff always
gives us the same result as taking two diffs and then canonizing.
[...]
I guess we're looking for two things. Canonicalness subsumes minimality, so
I'm not sure why we test for the latter. My guess is that it's because
minimality is useful on its own and easier to achieve.
Correct. Though I still don't know how to do it, but at least it seems
like it might be easier :-)
I confess I haven't run these tests yet -- is that easy to do?
Easiest thing is to bring them up in ghci and run them manually. I
generally get ghci going by doing cabal build -v and then copying the ghc
command line but replacing --make with --interactive.
Ganesh
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users
