Make the XmParser config test runtime more consistent
authorIustin Pop <iustin@google.com>
Thu, 28 Feb 2013 10:31:35 +0000 (11:31 +0100)
committerIustin Pop <iustin@google.com>
Mon, 4 Mar 2013 16:39:37 +0000 (17:39 +0100)
Currently, the test uses a frequency of 5 string/5 double/1 list for
generating Arbitrary instances of ListConfig. However, the list case
has simply a "choose (1, 20)" `vectorOf` arbitrary, which means it
could recurse forever.

Manually running only this test gives runtime as such:

- ~100-200ms: very often
- ~1-2s: often
- ~5s: rare
- ~20s: very rare (but I hit this when running < 30 times the test,
  so…)

On average, this makes this test one of the slowest ones, which is
annoying.

By changing to a sized generator, we can control the depth of the
recursion, ensuring that we have a consistent runtime: out of 100
runs, one is 229ms, one is 164ms, the other are 80-120ms.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

test/hs/Test/Ganeti/Hypervisor/Xen/XmParser.hs

index f6f7a31..f22b515 100644 (file)
@@ -48,17 +48,31 @@ import Ganeti.Hypervisor.Xen.XmParser
 
 -- * Arbitraries
 
--- | Arbitrary instance for generating configurations.
--- A completely arbitrary configuration would contain too many lists and its
--- size would be to big to be actually parsable in reasonable time.
--- This Arbitrary builds a random Config that is still of a reasonable size.
--- Avoid generating strings that might be interpreted as numbers.
+-- | Generator for 'ListConfig'.
+--
+-- A completely arbitrary configuration would contain too many lists
+-- and its size would be to big to be actually parsable in reasonable
+-- time. This generator builds a random Config that is still of a
+-- reasonable size, and it also Avoids generating strings that might
+-- be interpreted as numbers.
+genConfig :: Int -> Gen LispConfig
+genConfig 0 =
+  -- only terminal values for size 0
+  frequency [ (5, liftM LCString (genName `suchThat` (not . canBeNumber)))
+            , (5, liftM LCDouble arbitrary)
+            ]
+genConfig n =
+  -- for size greater than 0, allow "some" lists
+  frequency [ (5, liftM LCString (resize n genName `suchThat`
+                                  (not . canBeNumber)))
+            , (5, liftM LCDouble arbitrary)
+            , (1, liftM LCList (choose (1, n) >>=
+                                (\n' -> vectorOf n' (genConfig $ n `div` n'))))
+            ]
+
+-- | Arbitrary instance for 'LispConfig' using 'genConfig'.
 instance Arbitrary LispConfig where
-  arbitrary = frequency
-    [ (5, liftM LCString (genName `suchThat` (not . canBeNumber)))
-    , (5, liftM LCDouble arbitrary)
-    , (1, liftM LCList (choose(1,20) >>= (`vectorOf` arbitrary)))
-    ]
+  arbitrary = sized genConfig
 
 -- | Determines conservatively whether a string could be a number.
 canBeNumber :: String -> Bool