Repository navigation
Improving correctness #95
Description
I'm quite impressed by (and a bit proud of!) the number of bugs that has been revealed through dhall's usage of prettyprinter!
I wonder how prettyprinter could rely less on the users to discover these bugs though.
In particular I wonder whether property tests could help – IMHO they've been quite effective in dhall!
For example I believe that the recent bugs could have been discovered by tests for the following properties:
- Unbounded layout of
grouped Line fails #91: The result of applyinglayout{Pretty,Smart} opts . groupto aDocthat doesn't containFail, should never containSFail. - For
removeTrailingWhitespace:- The (rendered) input and output should only differ by trailing spaces.
- The output should never contain trailing spaces.
removeTrailingWhitespaceis idempotent.
It's probably more difficult to come up with properties that would reveal unknown bugs, but it might be worth trying!
Activity
sjakobi commentedon Nov 4, 2019
Oh, I see that there is already an
Arbitraryinstance and a non-trivial property test:https://github.com/quchen/prettyprinter/blob/4d51362d7ea8bca0fe9e7ffec2ccd8fcd0065c0d/prettyprinter/test/Testsuite/Main.hs#L79-L99
sjakobi commentedon Nov 4, 2019
Here's a property for #83:
When layouting
f (align (vsep […, x, …, x, …])), both instances ofxshould have the same layout.quchen commentedon Nov 5, 2019
The main difficulty here is putting the properties into code, but I totally agree! The
Arbitraryinstance is not used very much right now, but I prefer Quickcheck to handwritten tests whenever possible.sjakobi commentedon Nov 6, 2019
Another property had come up in #89:
But
layoutCompactshould be included too, so we probably wantwith
layoutbeing one oflayout{Pretty,Smart,Compact}with arbitraryLayoutOptions.Run fusion property test with arbitrary layouters
Run fusion property test with arbitrary layouters
sjakobi commentedon Jan 23, 2020
@1Jajen1 mentioned that he has some effective property tests in his kotlin clone.
Maybe we can copy some of those…
1Jajen1 commentedon Jan 23, 2020
@sjakobi Not sure that will work much better, currently I literally just generate random elements of Doc (without Union/Fail to preserve invariants although you could call group on them randomly to produce those as well). Shrinking is also done manually for most of it as I too use a quickcheck derivative for tests. I'll change those tests to more hedgehog style tests later this week to check out how well integrated shrinking works, it seems to be the better choice if you want to preserve invariants on more complex datatypes without writing shrinkers manually.
sjakobi commentedon Jan 23, 2020
@1Jajen1 Well somehow your tests were able to detect the problems with the modified
group, right?Which properties did you check there?
1Jajen1 commentedon Jan 23, 2020
So what I have right now is basically just a few property tests: (The kotlin source for these is actually readable!)
This one for the group (passes around 100k tests just fine, so it might actually work^^):
https://github.com/1Jajen1/kotlin-pretty/blob/master/kotlin-pretty/src/test/kotlin/pretty/GroupTest.kt
https://github.com/1Jajen1/kotlin-pretty/blob/master/kotlin-pretty/src/test/kotlin/pretty/Generators.kt
https://github.com/1Jajen1/kotlin-pretty/blob/master/kotlin-pretty/src/test/kotlin/pretty/DocTest.kt
They actually find a bug in
flatten(did not handleLinecorrectly, only in the kotlin version).sjakobi commentedon Jan 24, 2020
#126 (comment) lists a bunch of tests from
dhall's testsuite, at least some of which we ought to adopt intoprettyprinter's testsuite…