Add a unit-test helper function

[ganeti-local] / htools / Ganeti / HTools / QC.hs

diff --git a/htools/Ganeti/HTools/QC.hs b/htools/Ganeti/HTools/QC.hs
index 9b130bc..732d744 100644 (file)
--- a/htools/Ganeti/HTools/QC.hs
+++ b/htools/Ganeti/HTools/QC.hs
@@ -1,10 +1,12 @@
-{-| Unittests for ganeti-htools
+{-# LANGUAGE TemplateHaskell #-}
+
+{-| Unittests for ganeti-htools.

 
 -}
 
 {-
 
 
 -}
 
 {-
 

-Copyright (C) 2009, 2010, 2011 Google Inc.
+Copyright (C) 2009, 2010, 2011, 2012 Google Inc.

 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
 
 This program is free software; you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by

@@ -24,20 +26,20 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
 -}
 
 module Ganeti.HTools.QC
 -}
 
 module Ganeti.HTools.QC

-    ( testUtils
-    , testPeerMap
-    , testContainer
-    , testInstance
-    , testNode
-    , testText
-    , testOpCodes
-    , testJobs
-    , testCluster
-    , testLoader
-    ) where
+  ( testUtils
+  , testPeerMap
+  , testContainer
+  , testInstance
+  , testNode
+  , testText
+  , testOpCodes
+  , testJobs
+  , testCluster
+  , testLoader
+  , testTypes
+  ) where

 
 import Test.QuickCheck
 
 import Test.QuickCheck

-import Test.QuickCheck.Batch

 import Data.List (findIndex, intercalate, nub, isPrefixOf)
 import Data.Maybe
 import Control.Monad
 import Data.List (findIndex, intercalate, nub, isPrefixOf)
 import Data.Maybe
 import Control.Monad

@@ -53,6 +55,7 @@ import qualified Ganeti.HTools.Container as Container
 import qualified Ganeti.HTools.ExtLoader
 import qualified Ganeti.HTools.IAlloc as IAlloc
 import qualified Ganeti.HTools.Instance as Instance
 import qualified Ganeti.HTools.ExtLoader
 import qualified Ganeti.HTools.IAlloc as IAlloc
 import qualified Ganeti.HTools.Instance as Instance

+import qualified Ganeti.HTools.JSON as JSON

 import qualified Ganeti.HTools.Loader as Loader
 import qualified Ganeti.HTools.Luxi
 import qualified Ganeti.HTools.Node as Node
 import qualified Ganeti.HTools.Loader as Loader
 import qualified Ganeti.HTools.Luxi
 import qualified Ganeti.HTools.Node as Node

@@ -66,24 +69,55 @@ import qualified Ganeti.HTools.Utils as Utils
 import qualified Ganeti.HTools.Version
 import qualified Ganeti.Constants as C
 
 import qualified Ganeti.HTools.Version
 import qualified Ganeti.Constants as C
 

+import qualified Ganeti.HTools.Program.Hail
+import qualified Ganeti.HTools.Program.Hbal
+import qualified Ganeti.HTools.Program.Hscan
+import qualified Ganeti.HTools.Program.Hspace
+
+import Ganeti.HTools.QCHelper (testSuite)
+

 -- * Constants
 
 -- * Constants
 

--- | Maximum memory (1TiB, somewhat random value)
+-- | Maximum memory (1TiB, somewhat random value).

 maxMem :: Int
 maxMem = 1024 * 1024
 
 maxMem :: Int
 maxMem = 1024 * 1024
 

--- | Maximum disk (8TiB, somewhat random value)
+-- | Maximum disk (8TiB, somewhat random value).

 maxDsk :: Int
 maxDsk = 1024 * 1024 * 8
 
 maxDsk :: Int
 maxDsk = 1024 * 1024 * 8
 

--- | Max CPUs (1024, somewhat random value)
+-- | Max CPUs (1024, somewhat random value).

 maxCpu :: Int
 maxCpu = 1024
 
 maxCpu :: Int
 maxCpu = 1024
 

+-- | Null iPolicy, and by null we mean very liberal.
+nullIPolicy = Types.IPolicy
+  { Types.iPolicyMinSpec = Types.ISpec { Types.iSpecMemorySize = 0
+                                       , Types.iSpecCpuCount   = 0
+                                       , Types.iSpecDiskSize   = 0
+                                       , Types.iSpecDiskCount  = 0
+                                       , Types.iSpecNicCount   = 0
+                                       }
+  , Types.iPolicyMaxSpec = Types.ISpec { Types.iSpecMemorySize = maxBound
+                                       , Types.iSpecCpuCount   = maxBound
+                                       , Types.iSpecDiskSize   = maxBound
+                                       , Types.iSpecDiskCount  = C.maxDisks
+                                       , Types.iSpecNicCount   = C.maxNics
+                                       }
+  , Types.iPolicyStdSpec = Types.ISpec { Types.iSpecMemorySize = Types.unitMem
+                                       , Types.iSpecCpuCount   = Types.unitCpu
+                                       , Types.iSpecDiskSize   = Types.unitDsk
+                                       , Types.iSpecDiskCount  = 1
+                                       , Types.iSpecNicCount   = 1
+                                       }
+  , Types.iPolicyDiskTemplates = [Types.DTDrbd8, Types.DTPlain]
+  }
+
+

 defGroup :: Group.Group
 defGroup = flip Group.setIdx 0 $
 defGroup :: Group.Group
 defGroup = flip Group.setIdx 0 $

-               Group.create "default" Utils.defaultGroupID
-                    Types.AllocPreferred
+             Group.create "default" Types.defaultGroupID Types.AllocPreferred
+                  nullIPolicy

 
 defGroupList :: Group.List
 defGroupList = Container.fromList [(Group.idx defGroup, defGroup)]
 
 defGroupList :: Group.List
 defGroupList = Container.fromList [(Group.idx defGroup, defGroup)]

@@ -93,33 +127,63 @@ defGroupAssoc = Data.Map.singleton (Group.uuid defGroup) (Group.idx defGroup)
 
 -- * Helper functions
 
 
 -- * Helper functions
 

--- | Simple checker for whether OpResult is fail or pass
+-- | Simple checker for whether OpResult is fail or pass.

 isFailure :: Types.OpResult a -> Bool
 isFailure (Types.OpFail _) = True
 isFailure _ = False
 
 isFailure :: Types.OpResult a -> Bool
 isFailure (Types.OpFail _) = True
 isFailure _ = False
 

--- | Update an instance to be smaller than a node
-setInstanceSmallerThanNode node inst =
-    inst { Instance.mem = Node.availMem node `div` 2
-         , Instance.dsk = Node.availDisk node `div` 2
-         , Instance.vcpus = Node.availCpu node `div` 2
-         }
+-- | Checks for equality with proper annotation.
+(==?) :: (Show a, Eq a) => a -> a -> Property
+(==?) x y = printTestCase
+            ("Expected equality, but '" ++
+             show x ++ "' /= '" ++ show y ++ "'") (x == y)
+infix 3 ==?

 
 

--- | Create an instance given its spec
+-- | Show a message and fail the test.
+failTest :: String -> Property
+failTest msg = printTestCase msg False
+
+-- | Update an instance to be smaller than a node.
+setInstanceSmallerThanNode node inst =
+  inst { Instance.mem = Node.availMem node `div` 2
+       , Instance.dsk = Node.availDisk node `div` 2
+       , Instance.vcpus = Node.availCpu node `div` 2
+       }
+
+-- | Check if an instance is smaller than a node.
+isInstanceSmallerThanNode node inst =
+  Instance.mem inst   <= Node.availMem node `div` 2 &&
+  Instance.dsk inst   <= Node.availDisk node `div` 2 &&
+  Instance.vcpus inst <= Node.availCpu node `div` 2
+
+-- | Create an instance given its spec.

 createInstance mem dsk vcpus =
 createInstance mem dsk vcpus =

-    Instance.create "inst-unnamed" mem dsk vcpus "running" [] True (-1) (-1)
+  Instance.create "inst-unnamed" mem dsk vcpus Types.Running [] True (-1) (-1)
+    Types.DTDrbd8

 
 

--- | Create a small cluster by repeating a node spec
+-- | Create a small cluster by repeating a node spec.

 makeSmallCluster :: Node.Node -> Int -> Node.List
 makeSmallCluster node count =
 makeSmallCluster :: Node.Node -> Int -> Node.List
 makeSmallCluster node count =

-    let fn = Node.buildPeers node Container.empty
-        namelst = map (\n -> (Node.name n, n)) (replicate count fn)
-        (_, nlst) = Loader.assignIndices namelst
-    in nlst
-
--- | Checks if a node is "big" enough
-isNodeBig :: Node.Node -> Int -> Bool
-isNodeBig node size = Node.availDisk node > size * Types.unitDsk
+  let origname = Node.name node
+      origalias = Node.alias node
+      nodes = map (\idx -> node { Node.name = origname ++ "-" ++ show idx
+                                , Node.alias = origalias ++ "-" ++ show idx })
+              [1..count]
+      fn = flip Node.buildPeers Container.empty
+      namelst = map (\n -> (Node.name n, fn n)) nodes
+      (_, nlst) = Loader.assignIndices namelst
+  in nlst
+
+-- | Make a small cluster, both nodes and instances.
+makeSmallEmptyCluster :: Node.Node -> Int -> Instance.Instance
+                      -> (Node.List, Instance.List, Instance.Instance)
+makeSmallEmptyCluster node count inst =
+  (makeSmallCluster node count, Container.empty,
+   setInstanceSmallerThanNode node inst)
+
+-- | Checks if a node is "big" enough.
+isNodeBig :: Int -> Node.Node -> Bool
+isNodeBig size node = Node.availDisk node > size * Types.unitDsk

                       && Node.availMem node > size * Types.unitMem
                       && Node.availCpu node > size * Types.unitCpu
 
                       && Node.availMem node > size * Types.unitMem
                       && Node.availCpu node > size * Types.unitCpu
 

@@ -127,7 +191,7 @@ canBalance :: Cluster.Table -> Bool -> Bool -> Bool -> Bool
 canBalance tbl dm im evac = isJust $ Cluster.tryBalance tbl dm im evac 0 0
 
 -- | Assigns a new fresh instance to a cluster; this is not
 canBalance tbl dm im evac = isJust $ Cluster.tryBalance tbl dm im evac 0 0
 
 -- | Assigns a new fresh instance to a cluster; this is not

--- allocation, so no resource checks are done
+-- allocation, so no resource checks are done.

 assignInstance :: Node.List -> Instance.List -> Instance.Instance ->
                   Types.Idx -> Types.Idx ->
                   (Node.List, Instance.List)
 assignInstance :: Node.List -> Instance.List -> Instance.Instance ->
                   Types.Idx -> Types.Idx ->
                   (Node.List, Instance.List)

@@ -135,8 +199,8 @@ assignInstance nl il inst pdx sdx =
   let pnode = Container.find pdx nl
       snode = Container.find sdx nl
       maxiidx = if Container.null il
   let pnode = Container.find pdx nl
       snode = Container.find sdx nl
       maxiidx = if Container.null il

-                then 0
-                else fst (Container.findMax il) + 1
+                  then 0
+                  else fst (Container.findMax il) + 1

       inst' = inst { Instance.idx = maxiidx,
                      Instance.pNode = pdx, Instance.sNode = sdx }
       pnode' = Node.setPri pnode inst'
       inst' = inst { Instance.idx = maxiidx,
                      Instance.pNode = pdx, Instance.sNode = sdx }
       pnode' = Node.setPri pnode inst'

@@ -147,15 +211,13 @@ assignInstance nl il inst pdx sdx =
 
 -- * Arbitrary instances
 
 
 -- * Arbitrary instances
 

--- copied from the introduction to quickcheck
-instance Arbitrary Char where
-    arbitrary = choose ('\32', '\128')
-
+-- | Defines a DNS name.

 newtype DNSChar = DNSChar { dnsGetChar::Char }
 newtype DNSChar = DNSChar { dnsGetChar::Char }

+

 instance Arbitrary DNSChar where
 instance Arbitrary DNSChar where

-    arbitrary = do
-      x <- elements (['a'..'z'] ++ ['0'..'9'] ++ "_-")
-      return (DNSChar x)
+  arbitrary = do
+    x <- elements (['a'..'z'] ++ ['0'..'9'] ++ "_-")
+    return (DNSChar x)

 
 getName :: Gen String
 getName = do
 
 getName :: Gen String
 getName = do

@@ -163,7 +225,6 @@ getName = do
   dn <- vector n::Gen [DNSChar]
   return (map dnsGetChar dn)
 
   dn <- vector n::Gen [DNSChar]
   return (map dnsGetChar dn)
 

-

 getFQDN :: Gen String
 getFQDN = do
   felem <- getName
 getFQDN :: Gen String
 getFQDN = do
   felem <- getName

@@ -172,49 +233,101 @@ getFQDN = do
   let frest' = map (map dnsGetChar) frest
   return (felem ++ "." ++ intercalate "." frest')
 
   let frest' = map (map dnsGetChar) frest
   return (felem ++ "." ++ intercalate "." frest')
 

+-- | Defines a tag type.
+newtype TagChar = TagChar { tagGetChar :: Char }
+
+-- | All valid tag chars. This doesn't need to match _exactly_
+-- Ganeti's own tag regex, just enough for it to be close.
+tagChar :: [Char]
+tagChar = ['a'..'z'] ++ ['A'..'Z'] ++ ['0'..'9'] ++ ".+*/:@-"
+
+instance Arbitrary TagChar where
+  arbitrary = do
+    c <- elements tagChar
+    return (TagChar c)
+
+-- | Generates a tag
+genTag :: Gen [TagChar]
+genTag = do
+  -- the correct value would be C.maxTagLen, but that's way too
+  -- verbose in unittests, and at the moment I don't see any possible
+  -- bugs with longer tags and the way we use tags in htools
+  n <- choose (1, 10)
+  vector n
+
+-- | Generates a list of tags (correctly upper bounded).
+genTags :: Gen [String]
+genTags = do
+  -- the correct value would be C.maxTagsPerObj, but per the comment
+  -- in genTag, we don't use tags enough in htools to warrant testing
+  -- such big values
+  n <- choose (0, 10::Int)
+  tags <- mapM (const genTag) [1..n]
+  return $ map (map tagGetChar) tags
+
+instance Arbitrary Types.InstanceStatus where
+    arbitrary = elements [minBound..maxBound]
+

 -- let's generate a random instance
 instance Arbitrary Instance.Instance where
 -- let's generate a random instance
 instance Arbitrary Instance.Instance where

-    arbitrary = do
-      name <- getFQDN
-      mem <- choose (0, maxMem)
-      dsk <- choose (0, maxDsk)
-      run_st <- elements [ C.inststErrorup
-                         , C.inststErrordown
-                         , C.inststAdmindown
-                         , C.inststNodedown
-                         , C.inststNodeoffline
-                         , C.inststRunning
-                         , "no_such_status1"
-                         , "no_such_status2"]
-      pn <- arbitrary
-      sn <- arbitrary
-      vcpus <- choose (0, maxCpu)
-      return $ Instance.create name mem dsk vcpus run_st [] True pn sn
+  arbitrary = do
+    name <- getFQDN
+    mem <- choose (0, maxMem)
+    dsk <- choose (0, maxDsk)
+    run_st <- arbitrary
+    pn <- arbitrary
+    sn <- arbitrary
+    vcpus <- choose (0, maxCpu)
+    return $ Instance.create name mem dsk vcpus run_st [] True pn sn
+              Types.DTDrbd8
+
+-- | Generas an arbitrary node based on sizing information.
+genNode :: Maybe Int -- ^ Minimum node size in terms of units
+        -> Maybe Int -- ^ Maximum node size (when Nothing, bounded
+                     -- just by the max... constants)
+        -> Gen Node.Node
+genNode min_multiplier max_multiplier = do
+  let (base_mem, base_dsk, base_cpu) =
+        case min_multiplier of
+          Just mm -> (mm * Types.unitMem,
+                      mm * Types.unitDsk,
+                      mm * Types.unitCpu)
+          Nothing -> (0, 0, 0)
+      (top_mem, top_dsk, top_cpu)  =
+        case max_multiplier of
+          Just mm -> (mm * Types.unitMem,
+                      mm * Types.unitDsk,
+                      mm * Types.unitCpu)
+          Nothing -> (maxMem, maxDsk, maxCpu)
+  name  <- getFQDN
+  mem_t <- choose (base_mem, top_mem)
+  mem_f <- choose (base_mem, mem_t)
+  mem_n <- choose (0, mem_t - mem_f)
+  dsk_t <- choose (base_dsk, top_dsk)
+  dsk_f <- choose (base_dsk, dsk_t)
+  cpu_t <- choose (base_cpu, top_cpu)
+  offl  <- arbitrary
+  let n = Node.create name (fromIntegral mem_t) mem_n mem_f
+          (fromIntegral dsk_t) dsk_f (fromIntegral cpu_t) offl 0
+      n' = Node.setPolicy nullIPolicy n
+  return $ Node.buildPeers n' Container.empty
+
+-- | Helper function to generate a sane node.
+genOnlineNode :: Gen Node.Node
+genOnlineNode = do
+  arbitrary `suchThat` (\n -> not (Node.offline n) &&
+                              not (Node.failN1 n) &&
+                              Node.availDisk n > 0 &&
+                              Node.availMem n > 0 &&
+                              Node.availCpu n > 0)

 
 -- and a random node
 instance Arbitrary Node.Node where
 
 -- and a random node
 instance Arbitrary Node.Node where

-    arbitrary = do
-      name <- getFQDN
-      mem_t <- choose (0, maxMem)
-      mem_f <- choose (0, mem_t)
-      mem_n <- choose (0, mem_t - mem_f)
-      dsk_t <- choose (0, maxDsk)
-      dsk_f <- choose (0, dsk_t)
-      cpu_t <- choose (0, maxCpu)
-      offl <- arbitrary
-      let n = Node.create name (fromIntegral mem_t) mem_n mem_f
-              (fromIntegral dsk_t) dsk_f (fromIntegral cpu_t) offl
-              0
-          n' = Node.buildPeers n Container.empty
-      return n'
+  arbitrary = genNode Nothing Nothing

 
 -- replace disks
 instance Arbitrary OpCodes.ReplaceDisksMode where
 
 -- replace disks
 instance Arbitrary OpCodes.ReplaceDisksMode where

-  arbitrary = elements [ OpCodes.ReplaceOnPrimary
-                       , OpCodes.ReplaceOnSecondary
-                       , OpCodes.ReplaceNewSecondary
-                       , OpCodes.ReplaceAuto
-                       ]
+  arbitrary = elements [minBound..maxBound]

 
 instance Arbitrary OpCodes.OpCode where
   arbitrary = do
 
 instance Arbitrary OpCodes.OpCode where
   arbitrary = do

@@ -223,18 +336,19 @@ instance Arbitrary OpCodes.OpCode where
                       , "OP_INSTANCE_FAILOVER"
                       , "OP_INSTANCE_MIGRATE"
                       ]
                       , "OP_INSTANCE_FAILOVER"
                       , "OP_INSTANCE_MIGRATE"
                       ]

-    (case op_id of
-        "OP_TEST_DELAY" ->
-          liftM3 OpCodes.OpTestDelay arbitrary arbitrary arbitrary
-        "OP_INSTANCE_REPLACE_DISKS" ->
-          liftM5 OpCodes.OpInstanceReplaceDisks arbitrary arbitrary
+    case op_id of
+      "OP_TEST_DELAY" ->
+        liftM3 OpCodes.OpTestDelay arbitrary arbitrary arbitrary
+      "OP_INSTANCE_REPLACE_DISKS" ->
+        liftM5 OpCodes.OpInstanceReplaceDisks arbitrary arbitrary

           arbitrary arbitrary arbitrary
           arbitrary arbitrary arbitrary

-        "OP_INSTANCE_FAILOVER" ->
-          liftM2 OpCodes.OpInstanceFailover arbitrary arbitrary
-        "OP_INSTANCE_MIGRATE" ->
-          liftM4 OpCodes.OpInstanceMigrate arbitrary arbitrary arbitrary
+      "OP_INSTANCE_FAILOVER" ->
+        liftM3 OpCodes.OpInstanceFailover arbitrary arbitrary

           arbitrary
           arbitrary

-        _ -> fail "Wrong opcode")
+      "OP_INSTANCE_MIGRATE" ->
+        liftM5 OpCodes.OpInstanceMigrate arbitrary arbitrary
+          arbitrary arbitrary arbitrary
+      _ -> fail "Wrong opcode"

 
 instance Arbitrary Jobs.OpStatus where
   arbitrary = elements [minBound..maxBound]
 
 instance Arbitrary Jobs.OpStatus where
   arbitrary = elements [minBound..maxBound]

@@ -242,80 +356,203 @@ instance Arbitrary Jobs.OpStatus where
 instance Arbitrary Jobs.JobStatus where
   arbitrary = elements [minBound..maxBound]
 
 instance Arbitrary Jobs.JobStatus where
   arbitrary = elements [minBound..maxBound]
 

+newtype SmallRatio = SmallRatio Double deriving Show
+instance Arbitrary SmallRatio where
+  arbitrary = do
+    v <- choose (0, 1)
+    return $ SmallRatio v
+
+instance Arbitrary Types.AllocPolicy where
+  arbitrary = elements [minBound..maxBound]
+
+instance Arbitrary Types.DiskTemplate where
+  arbitrary = elements [minBound..maxBound]
+
+instance Arbitrary Types.FailMode where
+  arbitrary = elements [minBound..maxBound]
+
+instance Arbitrary Types.EvacMode where
+  arbitrary = elements [minBound..maxBound]
+
+instance Arbitrary a => Arbitrary (Types.OpResult a) where
+  arbitrary = arbitrary >>= \c ->
+              if c
+                then liftM Types.OpGood arbitrary
+                else liftM Types.OpFail arbitrary
+
+instance Arbitrary Types.ISpec where
+  arbitrary = do
+    mem <- arbitrary::Gen (NonNegative Int)
+    dsk_c <- arbitrary::Gen (NonNegative Int)
+    dsk_s <- arbitrary::Gen (NonNegative Int)
+    cpu <- arbitrary::Gen (NonNegative Int)
+    nic <- arbitrary::Gen (NonNegative Int)
+    return Types.ISpec { Types.iSpecMemorySize = fromIntegral mem
+                       , Types.iSpecCpuCount   = fromIntegral cpu
+                       , Types.iSpecDiskSize   = fromIntegral dsk_s
+                       , Types.iSpecDiskCount  = fromIntegral dsk_c
+                       , Types.iSpecNicCount   = fromIntegral nic
+                       }
+
+-- | Helper function to check whether a spec is LTE than another
+iSpecSmaller :: Types.ISpec -> Types.ISpec -> Bool
+iSpecSmaller imin imax =
+  Types.iSpecMemorySize imin <= Types.iSpecMemorySize imax &&
+  Types.iSpecCpuCount imin   <= Types.iSpecCpuCount imax &&
+  Types.iSpecDiskSize imin   <= Types.iSpecDiskSize imax &&
+  Types.iSpecDiskCount imin  <= Types.iSpecDiskCount imax &&
+  Types.iSpecNicCount imin   <= Types.iSpecNicCount imax
+
+instance Arbitrary Types.IPolicy where
+  arbitrary = do
+    imin <- arbitrary
+    istd <- arbitrary `suchThat` (iSpecSmaller imin)
+    imax <- arbitrary `suchThat` (iSpecSmaller istd)
+    dts  <- arbitrary
+    return Types.IPolicy { Types.iPolicyMinSpec = imin
+                         , Types.iPolicyStdSpec = istd
+                         , Types.iPolicyMaxSpec = imax
+                         , Types.iPolicyDiskTemplates = dts
+                         }
+

 -- * Actual tests
 
 -- * Actual tests
 

--- If the list is not just an empty element, and if the elements do
--- not contain commas, then join+split should be idepotent
-prop_Utils_commaJoinSplit lst = lst /= [""] &&
-                                all (not . elem ',') lst ==>
-                                Utils.sepSplit ',' (Utils.commaJoin lst) == lst
--- Split and join should always be idempotent
-prop_Utils_commaSplitJoin s = Utils.commaJoin (Utils.sepSplit ',' s) == s
+-- ** Utils tests
+
+-- | If the list is not just an empty element, and if the elements do
+-- not contain commas, then join+split should be idempotent.
+prop_Utils_commaJoinSplit =
+  forAll (arbitrary `suchThat`
+          (\l -> l /= [""] && all (notElem ',') l )) $ \lst ->
+  Utils.sepSplit ',' (Utils.commaJoin lst) ==? lst
+
+-- | Split and join should always be idempotent.
+prop_Utils_commaSplitJoin s =
+  Utils.commaJoin (Utils.sepSplit ',' s) ==? s

 
 -- | fromObjWithDefault, we test using the Maybe monad and an integer
 
 -- | fromObjWithDefault, we test using the Maybe monad and an integer

--- value
+-- value.

 prop_Utils_fromObjWithDefault def_value random_key =
 prop_Utils_fromObjWithDefault def_value random_key =

-    -- a missing key will be returned with the default
-    Utils.fromObjWithDefault [] random_key def_value == Just def_value &&
-    -- a found key will be returned as is, not with default
-    Utils.fromObjWithDefault [(random_key, J.showJSON def_value)]
-         random_key (def_value+1) == Just def_value
-        where _types = (def_value :: Integer)
-
-testUtils =
-  [ run prop_Utils_commaJoinSplit
-  , run prop_Utils_commaSplitJoin
-  , run prop_Utils_fromObjWithDefault
-  ]
-
--- | Make sure add is idempotent
+  -- a missing key will be returned with the default
+  JSON.fromObjWithDefault [] random_key def_value == Just def_value &&
+  -- a found key will be returned as is, not with default
+  JSON.fromObjWithDefault [(random_key, J.showJSON def_value)]
+       random_key (def_value+1) == Just def_value
+    where _types = def_value :: Integer
+
+-- | Test that functional if' behaves like the syntactic sugar if.
+prop_Utils_if'if :: Bool -> Int -> Int -> Gen Prop
+prop_Utils_if'if cnd a b =
+  Utils.if' cnd a b ==? if cnd then a else b
+
+-- | Test basic select functionality
+prop_Utils_select :: Int      -- ^ Default result
+                  -> [Int]    -- ^ List of False values
+                  -> [Int]    -- ^ List of True values
+                  -> Gen Prop -- ^ Test result
+prop_Utils_select def lst1 lst2 =
+  Utils.select def (flist ++ tlist) ==? expectedresult
+    where expectedresult = Utils.if' (null lst2) def (head lst2)
+          flist = zip (repeat False) lst1
+          tlist = zip (repeat True)  lst2
+
+-- | Test basic select functionality with undefined default
+prop_Utils_select_undefd :: [Int]            -- ^ List of False values
+                         -> NonEmptyList Int -- ^ List of True values
+                         -> Gen Prop         -- ^ Test result
+prop_Utils_select_undefd lst1 (NonEmpty lst2) =
+  Utils.select undefined (flist ++ tlist) ==? head lst2
+    where flist = zip (repeat False) lst1
+          tlist = zip (repeat True)  lst2
+
+-- | Test basic select functionality with undefined list values
+prop_Utils_select_undefv :: [Int]            -- ^ List of False values
+                         -> NonEmptyList Int -- ^ List of True values
+                         -> Gen Prop         -- ^ Test result
+prop_Utils_select_undefv lst1 (NonEmpty lst2) =
+  Utils.select undefined cndlist ==? head lst2
+    where flist = zip (repeat False) lst1
+          tlist = zip (repeat True)  lst2
+          cndlist = flist ++ tlist ++ [undefined]
+
+prop_Utils_parseUnit (NonNegative n) =
+  Utils.parseUnit (show n) == Types.Ok n &&
+  Utils.parseUnit (show n ++ "m") == Types.Ok n &&
+  (case Utils.parseUnit (show n ++ "M") of
+     Types.Ok m -> if n > 0
+                     then m < n  -- for positive values, X MB is < than X MiB
+                     else m == 0 -- but for 0, 0 MB == 0 MiB
+     Types.Bad _ -> False) &&
+  Utils.parseUnit (show n ++ "g") == Types.Ok (n*1024) &&
+  Utils.parseUnit (show n ++ "t") == Types.Ok (n*1048576) &&
+  Types.isBad (Utils.parseUnit (show n ++ "x")::Types.Result Int)
+    where _types = n::Int
+
+-- | Test list for the Utils module.
+testSuite "Utils"
+            [ 'prop_Utils_commaJoinSplit
+            , 'prop_Utils_commaSplitJoin
+            , 'prop_Utils_fromObjWithDefault
+            , 'prop_Utils_if'if
+            , 'prop_Utils_select
+            , 'prop_Utils_select_undefd
+            , 'prop_Utils_select_undefv
+            , 'prop_Utils_parseUnit
+            ]
+
+-- ** PeerMap tests
+
+-- | Make sure add is idempotent.

 prop_PeerMap_addIdempotent pmap key em =
 prop_PeerMap_addIdempotent pmap key em =

-    fn puniq == fn (fn puniq)
+  fn puniq ==? fn (fn puniq)

     where _types = (pmap::PeerMap.PeerMap,
                     key::PeerMap.Key, em::PeerMap.Elem)
           fn = PeerMap.add key em
           puniq = PeerMap.accumArray const pmap
 
     where _types = (pmap::PeerMap.PeerMap,
                     key::PeerMap.Key, em::PeerMap.Elem)
           fn = PeerMap.add key em
           puniq = PeerMap.accumArray const pmap
 

--- | Make sure remove is idempotent
+-- | Make sure remove is idempotent.

 prop_PeerMap_removeIdempotent pmap key =
 prop_PeerMap_removeIdempotent pmap key =

-    fn puniq == fn (fn puniq)
+  fn puniq ==? fn (fn puniq)

     where _types = (pmap::PeerMap.PeerMap, key::PeerMap.Key)
           fn = PeerMap.remove key
           puniq = PeerMap.accumArray const pmap
 
     where _types = (pmap::PeerMap.PeerMap, key::PeerMap.Key)
           fn = PeerMap.remove key
           puniq = PeerMap.accumArray const pmap
 

--- | Make sure a missing item returns 0
+-- | Make sure a missing item returns 0.

 prop_PeerMap_findMissing pmap key =
 prop_PeerMap_findMissing pmap key =

-    PeerMap.find key (PeerMap.remove key puniq) == 0
+  PeerMap.find key (PeerMap.remove key puniq) ==? 0

     where _types = (pmap::PeerMap.PeerMap, key::PeerMap.Key)
           puniq = PeerMap.accumArray const pmap
 
     where _types = (pmap::PeerMap.PeerMap, key::PeerMap.Key)
           puniq = PeerMap.accumArray const pmap
 

--- | Make sure an added item is found
+-- | Make sure an added item is found.

 prop_PeerMap_addFind pmap key em =
 prop_PeerMap_addFind pmap key em =

-    PeerMap.find key (PeerMap.add key em puniq) == em
+  PeerMap.find key (PeerMap.add key em puniq) ==? em

     where _types = (pmap::PeerMap.PeerMap,
                     key::PeerMap.Key, em::PeerMap.Elem)
           puniq = PeerMap.accumArray const pmap
 
     where _types = (pmap::PeerMap.PeerMap,
                     key::PeerMap.Key, em::PeerMap.Elem)
           puniq = PeerMap.accumArray const pmap
 

--- | Manual check that maxElem returns the maximum indeed, or 0 for null
+-- | Manual check that maxElem returns the maximum indeed, or 0 for null.

 prop_PeerMap_maxElem pmap =
 prop_PeerMap_maxElem pmap =

-    PeerMap.maxElem puniq == if null puniq then 0
-                             else (maximum . snd . unzip) puniq
+  PeerMap.maxElem puniq ==? if null puniq then 0
+                              else (maximum . snd . unzip) puniq

     where _types = pmap::PeerMap.PeerMap
           puniq = PeerMap.accumArray const pmap
 
     where _types = pmap::PeerMap.PeerMap
           puniq = PeerMap.accumArray const pmap
 

-testPeerMap =
-    [ run prop_PeerMap_addIdempotent
-    , run prop_PeerMap_removeIdempotent
-    , run prop_PeerMap_maxElem
-    , run prop_PeerMap_addFind
-    , run prop_PeerMap_findMissing
-    ]
+-- | List of tests for the PeerMap module.
+testSuite "PeerMap"
+            [ 'prop_PeerMap_addIdempotent
+            , 'prop_PeerMap_removeIdempotent
+            , 'prop_PeerMap_maxElem
+            , 'prop_PeerMap_addFind
+            , 'prop_PeerMap_findMissing
+            ]

 
 

--- Container tests
+-- ** Container tests

 
 

+-- we silence the following due to hlint bug fixed in later versions
+{-# ANN prop_Container_addTwo "HLint: ignore Avoid lambda" #-}

 prop_Container_addTwo cdata i1 i2 =
 prop_Container_addTwo cdata i1 i2 =

-    fn i1 i2 cont == fn i2 i1 cont &&
-       fn i1 i2 cont == fn i1 i2 (fn i1 i2 cont)
+  fn i1 i2 cont == fn i2 i1 cont &&
+  fn i1 i2 cont == fn i1 i2 (fn i1 i2 cont)

     where _types = (cdata::[Int],
                     i1::Int, i2::Int)
           cont = foldl (\c x -> Container.add x x c) Container.empty cdata
     where _types = (cdata::[Int],
                     i1::Int, i2::Int)
           cont = foldl (\c x -> Container.add x x c) Container.empty cdata

@@ -324,18 +561,18 @@ prop_Container_addTwo cdata i1 i2 =
 prop_Container_nameOf node =
   let nl = makeSmallCluster node 1
       fnode = head (Container.elems nl)
 prop_Container_nameOf node =
   let nl = makeSmallCluster node 1
       fnode = head (Container.elems nl)

-  in Container.nameOf nl (Node.idx fnode) == Node.name fnode
+  in Container.nameOf nl (Node.idx fnode) ==? Node.name fnode

 
 

--- We test that in a cluster, given a random node, we can find it by
+-- | We test that in a cluster, given a random node, we can find it by

 -- its name and alias, as long as all names and aliases are unique,
 -- its name and alias, as long as all names and aliases are unique,

--- and that we fail to find a non-existing name
+-- and that we fail to find a non-existing name.

 prop_Container_findByName node othername =
   forAll (choose (1, 20)) $ \ cnt ->
   forAll (choose (0, cnt - 1)) $ \ fidx ->
   forAll (vector cnt) $ \ names ->
   (length . nub) (map fst names ++ map snd names) ==
   length names * 2 &&
 prop_Container_findByName node othername =
   forAll (choose (1, 20)) $ \ cnt ->
   forAll (choose (0, cnt - 1)) $ \ fidx ->
   forAll (vector cnt) $ \ names ->
   (length . nub) (map fst names ++ map snd names) ==
   length names * 2 &&

-  not (othername `elem` (map fst names ++ map snd names)) ==>
+  othername `notElem` (map fst names ++ map snd names) ==>

   let nl = makeSmallCluster node cnt
       nodes = Container.elems nl
       nodes' = map (\((name, alias), nn) -> (Node.idx nn,
   let nl = makeSmallCluster node cnt
       nodes = Container.elems nl
       nodes' = map (\((name, alias), nn) -> (Node.idx nn,

@@ -346,326 +583,389 @@ prop_Container_findByName node othername =
       target = snd (nodes' !! fidx)
   in Container.findByName nl' (Node.name target) == Just target &&
      Container.findByName nl' (Node.alias target) == Just target &&
       target = snd (nodes' !! fidx)
   in Container.findByName nl' (Node.name target) == Just target &&
      Container.findByName nl' (Node.alias target) == Just target &&

-     Container.findByName nl' othername == Nothing
+     isNothing (Container.findByName nl' othername)
+
+testSuite "Container"
+            [ 'prop_Container_addTwo
+            , 'prop_Container_nameOf
+            , 'prop_Container_findByName
+            ]

 
 

-testContainer =
-    [ run prop_Container_addTwo
-    , run prop_Container_nameOf
-    , run prop_Container_findByName
-    ]
+-- ** Instance tests

 
 -- Simple instance tests, we only have setter/getters
 
 prop_Instance_creat inst =
 
 -- Simple instance tests, we only have setter/getters
 
 prop_Instance_creat inst =

-    Instance.name inst == Instance.alias inst
+  Instance.name inst ==? Instance.alias inst

 
 prop_Instance_setIdx inst idx =
 
 prop_Instance_setIdx inst idx =

-    Instance.idx (Instance.setIdx inst idx) == idx
+  Instance.idx (Instance.setIdx inst idx) ==? idx

     where _types = (inst::Instance.Instance, idx::Types.Idx)
 
 prop_Instance_setName inst name =
     where _types = (inst::Instance.Instance, idx::Types.Idx)
 
 prop_Instance_setName inst name =

-    Instance.name newinst == name &&
-    Instance.alias newinst == name
+  Instance.name newinst == name &&
+  Instance.alias newinst == name

     where _types = (inst::Instance.Instance, name::String)
           newinst = Instance.setName inst name
 
 prop_Instance_setAlias inst name =
     where _types = (inst::Instance.Instance, name::String)
           newinst = Instance.setName inst name
 
 prop_Instance_setAlias inst name =

-    Instance.name newinst == Instance.name inst &&
-    Instance.alias newinst == name
+  Instance.name newinst == Instance.name inst &&
+  Instance.alias newinst == name

     where _types = (inst::Instance.Instance, name::String)
           newinst = Instance.setAlias inst name
 
 prop_Instance_setPri inst pdx =
     where _types = (inst::Instance.Instance, name::String)
           newinst = Instance.setAlias inst name
 
 prop_Instance_setPri inst pdx =

-    Instance.pNode (Instance.setPri inst pdx) == pdx
+  Instance.pNode (Instance.setPri inst pdx) ==? pdx

     where _types = (inst::Instance.Instance, pdx::Types.Ndx)
 
 prop_Instance_setSec inst sdx =
     where _types = (inst::Instance.Instance, pdx::Types.Ndx)
 
 prop_Instance_setSec inst sdx =

-    Instance.sNode (Instance.setSec inst sdx) == sdx
+  Instance.sNode (Instance.setSec inst sdx) ==? sdx

     where _types = (inst::Instance.Instance, sdx::Types.Ndx)
 
 prop_Instance_setBoth inst pdx sdx =
     where _types = (inst::Instance.Instance, sdx::Types.Ndx)
 
 prop_Instance_setBoth inst pdx sdx =

-    Instance.pNode si == pdx && Instance.sNode si == sdx
+  Instance.pNode si == pdx && Instance.sNode si == sdx

     where _types = (inst::Instance.Instance, pdx::Types.Ndx, sdx::Types.Ndx)
           si = Instance.setBoth inst pdx sdx
 
     where _types = (inst::Instance.Instance, pdx::Types.Ndx, sdx::Types.Ndx)
           si = Instance.setBoth inst pdx sdx
 

-prop_Instance_runStatus_True inst =
-    let run_st = Instance.running inst
-        run_tx = Instance.runSt inst
-    in
-      run_tx `elem` Instance.runningStates ==> run_st
-
-prop_Instance_runStatus_False inst =
-    let run_st = Instance.running inst
-        run_tx = Instance.runSt inst
-    in
-      run_tx `notElem` Instance.runningStates ==> not run_st
-

 prop_Instance_shrinkMG inst =
 prop_Instance_shrinkMG inst =

-    Instance.mem inst >= 2 * Types.unitMem ==>
-        case Instance.shrinkByType inst Types.FailMem of
-          Types.Ok inst' ->
-              Instance.mem inst' == Instance.mem inst - Types.unitMem
-          _ -> False
+  Instance.mem inst >= 2 * Types.unitMem ==>
+    case Instance.shrinkByType inst Types.FailMem of
+      Types.Ok inst' -> Instance.mem inst' == Instance.mem inst - Types.unitMem
+      _ -> False

 
 prop_Instance_shrinkMF inst =
 
 prop_Instance_shrinkMF inst =

-    Instance.mem inst < 2 * Types.unitMem ==>
-        Types.isBad $ Instance.shrinkByType inst Types.FailMem
+  forAll (choose (0, 2 * Types.unitMem - 1)) $ \mem ->
+    let inst' = inst { Instance.mem = mem}
+    in Types.isBad $ Instance.shrinkByType inst' Types.FailMem

 
 prop_Instance_shrinkCG inst =
 
 prop_Instance_shrinkCG inst =

-    Instance.vcpus inst >= 2 * Types.unitCpu ==>
-        case Instance.shrinkByType inst Types.FailCPU of
-          Types.Ok inst' ->
-              Instance.vcpus inst' == Instance.vcpus inst - Types.unitCpu
-          _ -> False
+  Instance.vcpus inst >= 2 * Types.unitCpu ==>
+    case Instance.shrinkByType inst Types.FailCPU of
+      Types.Ok inst' ->
+        Instance.vcpus inst' == Instance.vcpus inst - Types.unitCpu
+      _ -> False

 
 prop_Instance_shrinkCF inst =
 
 prop_Instance_shrinkCF inst =

-    Instance.vcpus inst < 2 * Types.unitCpu ==>
-        Types.isBad $ Instance.shrinkByType inst Types.FailCPU
+  forAll (choose (0, 2 * Types.unitCpu - 1)) $ \vcpus ->
+    let inst' = inst { Instance.vcpus = vcpus }
+    in Types.isBad $ Instance.shrinkByType inst' Types.FailCPU

 
 prop_Instance_shrinkDG inst =
 
 prop_Instance_shrinkDG inst =

-    Instance.dsk inst >= 2 * Types.unitDsk ==>
-        case Instance.shrinkByType inst Types.FailDisk of
-          Types.Ok inst' ->
-              Instance.dsk inst' == Instance.dsk inst - Types.unitDsk
-          _ -> False
+  Instance.dsk inst >= 2 * Types.unitDsk ==>
+    case Instance.shrinkByType inst Types.FailDisk of
+      Types.Ok inst' ->
+        Instance.dsk inst' == Instance.dsk inst - Types.unitDsk
+      _ -> False

 
 prop_Instance_shrinkDF inst =
 
 prop_Instance_shrinkDF inst =

-    Instance.dsk inst < 2 * Types.unitDsk ==>
-        Types.isBad $ Instance.shrinkByType inst Types.FailDisk
+  forAll (choose (0, 2 * Types.unitDsk - 1)) $ \dsk ->
+    let inst' = inst { Instance.dsk = dsk }
+    in Types.isBad $ Instance.shrinkByType inst' Types.FailDisk

 
 prop_Instance_setMovable inst m =
 
 prop_Instance_setMovable inst m =

-    Instance.movable inst' == m
+  Instance.movable inst' ==? m

     where inst' = Instance.setMovable inst m
 
     where inst' = Instance.setMovable inst m
 

-testInstance =
-    [ run prop_Instance_creat
-    , run prop_Instance_setIdx
-    , run prop_Instance_setName
-    , run prop_Instance_setAlias
-    , run prop_Instance_setPri
-    , run prop_Instance_setSec
-    , run prop_Instance_setBoth
-    , run prop_Instance_runStatus_True
-    , run prop_Instance_runStatus_False
-    , run prop_Instance_shrinkMG
-    , run prop_Instance_shrinkMF
-    , run prop_Instance_shrinkCG
-    , run prop_Instance_shrinkCF
-    , run prop_Instance_shrinkDG
-    , run prop_Instance_shrinkDF
-    , run prop_Instance_setMovable
-    ]
+testSuite "Instance"
+            [ 'prop_Instance_creat
+            , 'prop_Instance_setIdx
+            , 'prop_Instance_setName
+            , 'prop_Instance_setAlias
+            , 'prop_Instance_setPri
+            , 'prop_Instance_setSec
+            , 'prop_Instance_setBoth
+            , 'prop_Instance_shrinkMG
+            , 'prop_Instance_shrinkMF
+            , 'prop_Instance_shrinkCG
+            , 'prop_Instance_shrinkCF
+            , 'prop_Instance_shrinkDG
+            , 'prop_Instance_shrinkDF
+            , 'prop_Instance_setMovable
+            ]
+
+-- ** Text backend tests

 
 -- Instance text loader tests
 
 
 -- Instance text loader tests
 

-prop_Text_Load_Instance name mem dsk vcpus status pnode snode pdx sdx autobal =
-    not (null pnode) && pdx >= 0 && sdx >= 0 ==>
-    let vcpus_s = show vcpus
-        dsk_s = show dsk
-        mem_s = show mem
-        rsdx = if pdx == sdx
-               then sdx + 1
-               else sdx
-        ndx = if null snode
+prop_Text_Load_Instance name mem dsk vcpus status
+                        (NonEmpty pnode) snode
+                        (NonNegative pdx) (NonNegative sdx) autobal dt =
+  pnode /= snode && pdx /= sdx ==>
+  let vcpus_s = show vcpus
+      dsk_s = show dsk
+      mem_s = show mem
+      status_s = Types.instanceStatusToRaw status
+      ndx = if null snode

               then [(pnode, pdx)]
               then [(pnode, pdx)]

-              else [(pnode, pdx), (snode, rsdx)]
-        nl = Data.Map.fromList ndx
-        tags = ""
-        sbal = if autobal then "Y" else "N"
-        inst = Text.loadInst nl
-               [name, mem_s, dsk_s, vcpus_s, status,
-                sbal, pnode, snode, tags]:: Maybe (String, Instance.Instance)
-        fail1 = Text.loadInst nl
-               [name, mem_s, dsk_s, vcpus_s, status,
-                sbal, pnode, pnode, tags]:: Maybe (String, Instance.Instance)
-        _types = ( name::String, mem::Int, dsk::Int
-                 , vcpus::Int, status::String
-                 , pnode::String, snode::String
-                 , pdx::Types.Ndx, sdx::Types.Ndx
-                 , autobal::Bool)
-    in
-      case inst of
-        Nothing -> False
-        Just (_, i) ->
-            (Instance.name i == name &&
-             Instance.vcpus i == vcpus &&
-             Instance.mem i == mem &&
-             Instance.pNode i == pdx &&
-             Instance.sNode i == (if null snode
-                                  then Node.noSecondary
-                                  else rsdx) &&
-             Instance.auto_balance i == autobal &&
-             isNothing fail1)
+              else [(pnode, pdx), (snode, sdx)]
+      nl = Data.Map.fromList ndx
+      tags = ""
+      sbal = if autobal then "Y" else "N"
+      sdt = Types.diskTemplateToRaw dt
+      inst = Text.loadInst nl
+             [name, mem_s, dsk_s, vcpus_s, status_s,
+              sbal, pnode, snode, sdt, tags]
+      fail1 = Text.loadInst nl
+              [name, mem_s, dsk_s, vcpus_s, status_s,
+               sbal, pnode, pnode, tags]
+      _types = ( name::String, mem::Int, dsk::Int
+               , vcpus::Int, status::Types.InstanceStatus
+               , snode::String
+               , autobal::Bool)
+  in case inst of
+       Types.Bad msg -> failTest $ "Failed to load instance: " ++ msg
+       Types.Ok (_, i) -> printTestCase "Mismatch in some field while\
+                                        \ loading the instance" $
+               Instance.name i == name &&
+               Instance.vcpus i == vcpus &&
+               Instance.mem i == mem &&
+               Instance.pNode i == pdx &&
+               Instance.sNode i == (if null snode
+                                      then Node.noSecondary
+                                      else sdx) &&
+               Instance.autoBalance i == autobal &&
+               Types.isBad fail1

 
 prop_Text_Load_InstanceFail ktn fields =
 
 prop_Text_Load_InstanceFail ktn fields =

-    length fields /= 9 ==>
+  length fields /= 10 ==>

     case Text.loadInst nl fields of
     case Text.loadInst nl fields of

-      Right _ -> False
-      Left msg -> isPrefixOf "Invalid/incomplete instance data: '" msg
+      Types.Ok _ -> failTest "Managed to load instance from invalid data"
+      Types.Bad msg -> printTestCase ("Unrecognised error message: " ++ msg) $
+                       "Invalid/incomplete instance data: '" `isPrefixOf` msg

     where nl = Data.Map.fromList ktn
 
 prop_Text_Load_Node name tm nm fm td fd tc fo =
     where nl = Data.Map.fromList ktn
 
 prop_Text_Load_Node name tm nm fm td fd tc fo =

-    let conv v = if v < 0
-                    then "?"
-                    else show v
-        tm_s = conv tm
-        nm_s = conv nm
-        fm_s = conv fm
-        td_s = conv td
-        fd_s = conv fd
-        tc_s = conv tc
-        fo_s = if fo
+  let conv v = if v < 0
+                 then "?"
+                 else show v
+      tm_s = conv tm
+      nm_s = conv nm
+      fm_s = conv fm
+      td_s = conv td
+      fd_s = conv fd
+      tc_s = conv tc
+      fo_s = if fo

                then "Y"
                else "N"
                then "Y"
                else "N"

-        any_broken = any (< 0) [tm, nm, fm, td, fd, tc]
-        gid = Group.uuid defGroup
-    in case Text.loadNode defGroupAssoc
-           [name, tm_s, nm_s, fm_s, td_s, fd_s, tc_s, fo_s, gid] of
-         Nothing -> False
-         Just (name', node) ->
-             if fo || any_broken
-             then Node.offline node
-             else Node.name node == name' && name' == name &&
-                  Node.alias node == name &&
-                  Node.tMem node == fromIntegral tm &&
-                  Node.nMem node == nm &&
-                  Node.fMem node == fm &&
-                  Node.tDsk node == fromIntegral td &&
-                  Node.fDsk node == fd &&
-                  Node.tCpu node == fromIntegral tc
+      any_broken = any (< 0) [tm, nm, fm, td, fd, tc]
+      gid = Group.uuid defGroup
+  in case Text.loadNode defGroupAssoc
+       [name, tm_s, nm_s, fm_s, td_s, fd_s, tc_s, fo_s, gid] of
+       Nothing -> False
+       Just (name', node) ->
+         if fo || any_broken
+           then Node.offline node
+           else Node.name node == name' && name' == name &&
+                Node.alias node == name &&
+                Node.tMem node == fromIntegral tm &&
+                Node.nMem node == nm &&
+                Node.fMem node == fm &&
+                Node.tDsk node == fromIntegral td &&
+                Node.fDsk node == fd &&
+                Node.tCpu node == fromIntegral tc

 
 prop_Text_Load_NodeFail fields =
 
 prop_Text_Load_NodeFail fields =

-    length fields /= 8 ==> isNothing $ Text.loadNode Data.Map.empty fields
+  length fields /= 8 ==> isNothing $ Text.loadNode Data.Map.empty fields

 
 prop_Text_NodeLSIdempotent node =
 
 prop_Text_NodeLSIdempotent node =

-    (Text.loadNode defGroupAssoc.
-         Utils.sepSplit '|' . Text.serializeNode defGroupList) n ==
-    Just (Node.name n, n)
+  (Text.loadNode defGroupAssoc.
+       Utils.sepSplit '|' . Text.serializeNode defGroupList) n ==
+  Just (Node.name n, n)

     -- override failN1 to what loadNode returns by default
     -- override failN1 to what loadNode returns by default

-    where n = node { Node.failN1 = True, Node.offline = False }
-
-testText =
-    [ run prop_Text_Load_Instance
-    , run prop_Text_Load_InstanceFail
-    , run prop_Text_Load_Node
-    , run prop_Text_Load_NodeFail
-    , run prop_Text_NodeLSIdempotent
-    ]
-
--- Node tests
+    where n = node { Node.failN1 = True, Node.offline = False
+                   , Node.iPolicy = Types.defIPolicy }
+
+prop_Text_ISpecIdempotent ispec =
+  case Text.loadISpec "dummy" . Utils.sepSplit ',' .
+       Text.serializeISpec $ ispec of
+    Types.Bad msg -> failTest $ "Failed to load ispec: " ++ msg
+    Types.Ok ispec' -> ispec ==? ispec'
+
+prop_Text_IPolicyIdempotent ipol =
+  case Text.loadIPolicy . Utils.sepSplit '|' $
+       Text.serializeIPolicy owner ipol of
+    Types.Bad msg -> failTest $ "Failed to load ispec: " ++ msg
+    Types.Ok res -> (owner, ipol) ==? res
+  where owner = "dummy"
+
+-- | This property, while being in the text tests, does more than just
+-- test end-to-end the serialisation and loading back workflow; it
+-- also tests the Loader.mergeData and the actuall
+-- Cluster.iterateAlloc (for well-behaving w.r.t. instance
+-- allocations, not for the business logic). As such, it's a quite
+-- complex and slow test, and that's the reason we restrict it to
+-- small cluster sizes.
+prop_Text_CreateSerialise =
+  forAll genTags $ \ctags ->
+  forAll (choose (1, 2)) $ \reqnodes ->
+  forAll (choose (1, 20)) $ \maxiter ->
+  forAll (choose (2, 10)) $ \count ->
+  forAll genOnlineNode $ \node ->
+  forAll (arbitrary `suchThat` isInstanceSmallerThanNode node) $ \inst ->
+  let inst' = Instance.setMovable inst $ Utils.if' (reqnodes == 2) True False
+      nl = makeSmallCluster node count
+  in case Cluster.genAllocNodes defGroupList nl reqnodes True >>= \allocn ->
+     Cluster.iterateAlloc nl Container.empty (Just maxiter) inst' allocn [] []
+     of
+       Types.Bad msg -> failTest $ "Failed to allocate: " ++ msg
+       Types.Ok (_, _, _, [], _) -> printTestCase
+                                    "Failed to allocate: no allocations" False
+       Types.Ok (_, nl', il', _, _) ->
+         let cdata = Loader.ClusterData defGroupList nl' il' ctags
+                     Types.defIPolicy
+             saved = Text.serializeCluster cdata
+         in case Text.parseData saved >>= Loader.mergeData [] [] [] [] of
+              Types.Bad msg -> failTest $ "Failed to load/merge: " ++ msg
+              Types.Ok (Loader.ClusterData gl2 nl2 il2 ctags2 cpol2) ->
+                ctags ==? ctags2 .&&.
+                Types.defIPolicy ==? cpol2 .&&.
+                il' ==? il2 .&&.
+                defGroupList ==? gl2 .&&.
+                nl' ==? nl2
+
+testSuite "Text"
+            [ 'prop_Text_Load_Instance
+            , 'prop_Text_Load_InstanceFail
+            , 'prop_Text_Load_Node
+            , 'prop_Text_Load_NodeFail
+            , 'prop_Text_NodeLSIdempotent
+            , 'prop_Text_ISpecIdempotent
+            , 'prop_Text_IPolicyIdempotent
+            , 'prop_Text_CreateSerialise
+            ]
+
+-- ** Node tests

 
 prop_Node_setAlias node name =
 
 prop_Node_setAlias node name =

-    Node.name newnode == Node.name node &&
-    Node.alias newnode == name
+  Node.name newnode == Node.name node &&
+  Node.alias newnode == name

     where _types = (node::Node.Node, name::String)
           newnode = Node.setAlias node name
 
 prop_Node_setOffline node status =
     where _types = (node::Node.Node, name::String)
           newnode = Node.setAlias node name
 
 prop_Node_setOffline node status =

-    Node.offline newnode == status
+  Node.offline newnode ==? status

     where newnode = Node.setOffline node status
 
 prop_Node_setXmem node xm =
     where newnode = Node.setOffline node status
 
 prop_Node_setXmem node xm =

-    Node.xMem newnode == xm
+  Node.xMem newnode ==? xm

     where newnode = Node.setXmem node xm
 
 prop_Node_setMcpu node mc =
     where newnode = Node.setXmem node xm
 
 prop_Node_setMcpu node mc =

-    Node.mCpu newnode == mc
+  Node.mCpu newnode ==? mc

     where newnode = Node.setMcpu node mc
 
     where newnode = Node.setMcpu node mc
 

--- | Check that an instance add with too high memory or disk will be rejected
-prop_Node_addPriFM node inst = Instance.mem inst >= Node.fMem node &&
-                               not (Node.failN1 node)
-                               ==>
-                               case Node.addPri node inst'' of
-                                 Types.OpFail Types.FailMem -> True
-                                 _ -> False
-    where _types = (node::Node.Node, inst::Instance.Instance)
-          inst' = setInstanceSmallerThanNode node inst
-          inst'' = inst' { Instance.mem = Instance.mem inst }
-
-prop_Node_addPriFD node inst = Instance.dsk inst >= Node.fDsk node &&
-                               not (Node.failN1 node)
-                               ==>
-                               case Node.addPri node inst'' of
-                                 Types.OpFail Types.FailDisk -> True
-                                 _ -> False
+-- | Check that an instance add with too high memory or disk will be
+-- rejected.
+prop_Node_addPriFM node inst =
+  Instance.mem inst >= Node.fMem node && not (Node.failN1 node) &&
+  not (Instance.instanceOffline inst) ==>
+  case Node.addPri node inst'' of
+    Types.OpFail Types.FailMem -> True
+    _ -> False
+  where _types = (node::Node.Node, inst::Instance.Instance)
+        inst' = setInstanceSmallerThanNode node inst
+        inst'' = inst' { Instance.mem = Instance.mem inst }
+
+prop_Node_addPriFD node inst =
+  Instance.dsk inst >= Node.fDsk node && not (Node.failN1 node) ==>
+    case Node.addPri node inst'' of
+      Types.OpFail Types.FailDisk -> True
+      _ -> False

     where _types = (node::Node.Node, inst::Instance.Instance)
           inst' = setInstanceSmallerThanNode node inst
           inst'' = inst' { Instance.dsk = Instance.dsk inst }
 
     where _types = (node::Node.Node, inst::Instance.Instance)
           inst' = setInstanceSmallerThanNode node inst
           inst'' = inst' { Instance.dsk = Instance.dsk inst }
 

-prop_Node_addPriFC node inst = Instance.vcpus inst > Node.availCpu node &&
-                               not (Node.failN1 node)
-                               ==>
-                               case Node.addPri node inst'' of
-                                 Types.OpFail Types.FailCPU -> True
-                                 _ -> False
+prop_Node_addPriFC node inst (Positive extra) =
+  not (Node.failN1 node) && not (Instance.instanceOffline inst) ==>
+      case Node.addPri node inst'' of
+        Types.OpFail Types.FailCPU -> True
+        _ -> False

     where _types = (node::Node.Node, inst::Instance.Instance)
           inst' = setInstanceSmallerThanNode node inst
     where _types = (node::Node.Node, inst::Instance.Instance)
           inst' = setInstanceSmallerThanNode node inst

-          inst'' = inst' { Instance.vcpus = Instance.vcpus inst }
+          inst'' = inst' { Instance.vcpus = Node.availCpu node + extra }

 
 

--- | Check that an instance add with too high memory or disk will be rejected
+-- | Check that an instance add with too high memory or disk will be
+-- rejected.

 prop_Node_addSec node inst pdx =
 prop_Node_addSec node inst pdx =

-    (Instance.mem inst >= (Node.fMem node - Node.rMem node) ||
-     Instance.dsk inst >= Node.fDsk node) &&
-    not (Node.failN1 node)
-    ==> isFailure (Node.addSec node inst pdx)
+  ((Instance.mem inst >= (Node.fMem node - Node.rMem node) &&
+    not (Instance.instanceOffline inst)) ||
+   Instance.dsk inst >= Node.fDsk node) &&
+  not (Node.failN1 node) ==>
+      isFailure (Node.addSec node inst pdx)

         where _types = (node::Node.Node, inst::Instance.Instance, pdx::Int)
 
         where _types = (node::Node.Node, inst::Instance.Instance, pdx::Int)
 

--- | Checks for memory reservation changes
-prop_Node_rMem node inst =
-    -- ab = auto_balance, nb = non-auto_balance
-    -- we use -1 as the primary node of the instance
-    let inst' = inst { Instance.pNode = (-1), Instance.auto_balance = True}
-        inst_ab = setInstanceSmallerThanNode node inst'
-        inst_nb = inst_ab { Instance.auto_balance = False }
-        -- now we have the two instances, identical except the
-        -- auto_balance attribute
-        orig_rmem = Node.rMem node
-        inst_idx = Instance.idx inst_ab
-        node_add_ab = Node.addSec node inst_ab (-1)
-        node_add_nb = Node.addSec node inst_nb (-1)
-        node_del_ab = liftM (flip Node.removeSec inst_ab) node_add_ab
-        node_del_nb = liftM (flip Node.removeSec inst_nb) node_add_nb
-    in case (node_add_ab, node_add_nb, node_del_ab, node_del_nb) of
-         (Types.OpGood a_ab, Types.OpGood a_nb,
-          Types.OpGood d_ab, Types.OpGood d_nb) ->
-             Node.rMem a_ab >  orig_rmem &&
-             Node.rMem a_ab - orig_rmem == Instance.mem inst_ab &&
-             Node.rMem a_nb == orig_rmem &&
-             Node.rMem d_ab == orig_rmem &&
-             Node.rMem d_nb == orig_rmem &&
-             -- this is not related to rMem, but as good a place to
-             -- test as any
-             inst_idx `elem` Node.sList a_ab &&
-             not (inst_idx `elem` Node.sList d_ab)
-         _ -> False
-
-newtype SmallRatio = SmallRatio Double deriving Show
-instance Arbitrary SmallRatio where
-    arbitrary = do
-      v <- choose (0, 1)
-      return $ SmallRatio v
-
--- | Check mdsk setting
+-- | Check that an offline instance with reasonable disk size can always
+-- be added.
+prop_Node_addPriOffline =
+  forAll (arbitrary `suchThat` ((> 0) . Node.fMem)) $ \node ->
+  forAll (arbitrary `suchThat`
+          (\ x ->  (Instance.dsk x  < Node.fDsk node) &&
+                   Instance.instanceOffline x)) $ \inst ->
+  case Node.addPri node inst of
+    Types.OpGood _ -> True
+    _ -> False
+
+prop_Node_addSecOffline pdx =
+  forAll (arbitrary `suchThat` ((> 0) . Node.fMem)) $ \node ->
+  forAll (arbitrary `suchThat`
+          (\ x ->  (Instance.dsk x  < Node.fDsk node) &&
+                   Instance.instanceOffline x)) $ \inst ->
+  case Node.addSec node inst pdx of
+    Types.OpGood _ -> True
+    _ -> False
+
+-- | Checks for memory reservation changes.
+prop_Node_rMem inst =
+  not (Instance.instanceOffline inst) ==>
+  forAll (arbitrary `suchThat` ((> Types.unitMem) . Node.fMem)) $ \node ->
+  -- ab = auto_balance, nb = non-auto_balance
+  -- we use -1 as the primary node of the instance
+  let inst' = inst { Instance.pNode = -1, Instance.autoBalance = True }
+      inst_ab = setInstanceSmallerThanNode node inst'
+      inst_nb = inst_ab { Instance.autoBalance = False }
+      -- now we have the two instances, identical except the
+      -- autoBalance attribute
+      orig_rmem = Node.rMem node
+      inst_idx = Instance.idx inst_ab
+      node_add_ab = Node.addSec node inst_ab (-1)
+      node_add_nb = Node.addSec node inst_nb (-1)
+      node_del_ab = liftM (`Node.removeSec` inst_ab) node_add_ab
+      node_del_nb = liftM (`Node.removeSec` inst_nb) node_add_nb
+  in case (node_add_ab, node_add_nb, node_del_ab, node_del_nb) of
+       (Types.OpGood a_ab, Types.OpGood a_nb,
+        Types.OpGood d_ab, Types.OpGood d_nb) ->
+         printTestCase "Consistency checks failed" $
+           Node.rMem a_ab >  orig_rmem &&
+           Node.rMem a_ab - orig_rmem == Instance.mem inst_ab &&
+           Node.rMem a_nb == orig_rmem &&
+           Node.rMem d_ab == orig_rmem &&
+           Node.rMem d_nb == orig_rmem &&
+           -- this is not related to rMem, but as good a place to
+           -- test as any
+           inst_idx `elem` Node.sList a_ab &&
+           inst_idx `notElem` Node.sList d_ab
+       x -> failTest $ "Failed to add/remove instances: " ++ show x
+
+-- | Check mdsk setting.

 prop_Node_setMdsk node mx =
 prop_Node_setMdsk node mx =

-    Node.loDsk node' >= 0 &&
-    fromIntegral (Node.loDsk node') <= Node.tDsk node &&
-    Node.availDisk node' >= 0 &&
-    Node.availDisk node' <= Node.fDsk node' &&
-    fromIntegral (Node.availDisk node') <= Node.tDsk node' &&
-    Node.mDsk node' == mx'
+  Node.loDsk node' >= 0 &&
+  fromIntegral (Node.loDsk node') <= Node.tDsk node &&
+  Node.availDisk node' >= 0 &&
+  Node.availDisk node' <= Node.fDsk node' &&
+  fromIntegral (Node.availDisk node') <= Node.tDsk node' &&
+  Node.mDsk node' == mx'

     where _types = (node::Node.Node, mx::SmallRatio)
           node' = Node.setMdsk node mx'
           SmallRatio mx' = mx
 
 -- Check tag maps
 prop_Node_tagMaps_idempotent tags =
     where _types = (node::Node.Node, mx::SmallRatio)
           node' = Node.setMdsk node mx'
           SmallRatio mx' = mx
 
 -- Check tag maps
 prop_Node_tagMaps_idempotent tags =

-    Node.delTags (Node.addTags m tags) tags == m
+  Node.delTags (Node.addTags m tags) tags ==? m

     where m = Data.Map.empty
 
 prop_Node_tagMaps_reject tags =
     where m = Data.Map.empty
 
 prop_Node_tagMaps_reject tags =

-    not (null tags) ==>
-    any (\t -> Node.rejectAddTags m [t]) tags
+  not (null tags) ==>
+  all (\t -> Node.rejectAddTags m [t]) tags

     where m = Node.addTags Data.Map.empty tags
 
 prop_Node_showField node =
     where m = Node.addTags Data.Map.empty tags
 
 prop_Node_showField node =

@@ -673,151 +973,136 @@ prop_Node_showField node =
   fst (Node.showHeader field) /= Types.unknownField &&
   Node.showField node field /= Types.unknownField
 
   fst (Node.showHeader field) /= Types.unknownField &&
   Node.showField node field /= Types.unknownField
 

-

 prop_Node_computeGroups nodes =
   let ng = Node.computeGroups nodes
       onlyuuid = map fst ng
   in length nodes == sum (map (length . snd) ng) &&
      all (\(guuid, ns) -> all ((== guuid) . Node.group) ns) ng &&
      length (nub onlyuuid) == length onlyuuid &&
 prop_Node_computeGroups nodes =
   let ng = Node.computeGroups nodes
       onlyuuid = map fst ng
   in length nodes == sum (map (length . snd) ng) &&
      all (\(guuid, ns) -> all ((== guuid) . Node.group) ns) ng &&
      length (nub onlyuuid) == length onlyuuid &&

-     if null nodes then True else not (null ng)
-
-testNode =
-    [ run prop_Node_setAlias
-    , run prop_Node_setOffline
-    , run prop_Node_setMcpu
-    , run prop_Node_setXmem
-    , run prop_Node_addPriFM
-    , run prop_Node_addPriFD
-    , run prop_Node_addPriFC
-    , run prop_Node_addSec
-    , run prop_Node_rMem
-    , run prop_Node_setMdsk
-    , run prop_Node_tagMaps_idempotent
-    , run prop_Node_tagMaps_reject
-    , run prop_Node_showField
-    , run prop_Node_computeGroups
-    ]
-
-
--- Cluster tests
-
--- | Check that the cluster score is close to zero for a homogeneous cluster
-prop_Score_Zero node count =
+     (null nodes || not (null ng))
+
+testSuite "Node"
+            [ 'prop_Node_setAlias
+            , 'prop_Node_setOffline
+            , 'prop_Node_setMcpu
+            , 'prop_Node_setXmem
+            , 'prop_Node_addPriFM
+            , 'prop_Node_addPriFD
+            , 'prop_Node_addPriFC
+            , 'prop_Node_addSec
+            , 'prop_Node_addPriOffline
+            , 'prop_Node_addSecOffline
+            , 'prop_Node_rMem
+            , 'prop_Node_setMdsk
+            , 'prop_Node_tagMaps_idempotent
+            , 'prop_Node_tagMaps_reject
+            , 'prop_Node_showField
+            , 'prop_Node_computeGroups
+            ]
+
+-- ** Cluster tests
+
+-- | Check that the cluster score is close to zero for a homogeneous
+-- cluster.
+prop_Score_Zero node =
+  forAll (choose (1, 1024)) $ \count ->

     (not (Node.offline node) && not (Node.failN1 node) && (count > 0) &&
      (Node.tDsk node > 0) && (Node.tMem node > 0)) ==>
     (not (Node.offline node) && not (Node.failN1 node) && (count > 0) &&
      (Node.tDsk node > 0) && (Node.tMem node > 0)) ==>

-    let fn = Node.buildPeers node Container.empty
-        nlst = zip [1..] $ replicate count fn::[(Types.Ndx, Node.Node)]
-        nl = Container.fromList nlst
-        score = Cluster.compCV nl
-    -- we can't say == 0 here as the floating point errors accumulate;
-    -- this should be much lower than the default score in CLI.hs
-    in score <= 1e-15
-
--- | Check that cluster stats are sane
-prop_CStats_sane node count =
-    (not (Node.offline node) && not (Node.failN1 node) && (count > 0) &&
-     (Node.availDisk node > 0) && (Node.availMem node > 0)) ==>
-    let fn = Node.buildPeers node Container.empty
-        nlst = zip [1..] $ replicate count fn::[(Types.Ndx, Node.Node)]
-        nl = Container.fromList nlst
-        cstats = Cluster.totalResources nl
-    in Cluster.csAdsk cstats >= 0 &&
-       Cluster.csAdsk cstats <= Cluster.csFdsk cstats
+  let fn = Node.buildPeers node Container.empty
+      nlst = replicate count fn
+      score = Cluster.compCVNodes nlst
+  -- we can't say == 0 here as the floating point errors accumulate;
+  -- this should be much lower than the default score in CLI.hs
+  in score <= 1e-12
+
+-- | Check that cluster stats are sane.
+prop_CStats_sane =
+  forAll (choose (1, 1024)) $ \count ->
+  forAll genOnlineNode $ \node ->
+  let fn = Node.buildPeers node Container.empty
+      nlst = zip [1..] $ replicate count fn::[(Types.Ndx, Node.Node)]
+      nl = Container.fromList nlst
+      cstats = Cluster.totalResources nl
+  in Cluster.csAdsk cstats >= 0 &&
+     Cluster.csAdsk cstats <= Cluster.csFdsk cstats

 
 -- | Check that one instance is allocated correctly, without
 
 -- | Check that one instance is allocated correctly, without

--- rebalances needed
-prop_ClusterAlloc_sane node inst =
-    forAll (choose (5, 20)) $ \count ->
-    not (Node.offline node)
-            && not (Node.failN1 node)
-            && Node.availDisk node > 0
-            && Node.availMem node > 0
-            ==>
-    let nl = makeSmallCluster node count
-        il = Container.empty
-        inst' = setInstanceSmallerThanNode node inst
-    in case Cluster.genAllocNodes defGroupList nl 2 True >>=
-       Cluster.tryAlloc nl il inst' of
-         Types.Bad _ -> False
-         Types.Ok as ->
-             case Cluster.asSolutions as of
-               [] -> False
-               (xnl, xi, _, cv):[] ->
-                   let il' = Container.add (Instance.idx xi) xi il
-                       tbl = Cluster.Table xnl il' cv []
-                   in not (canBalance tbl True True False)
-               _ -> False
+-- rebalances needed.
+prop_ClusterAlloc_sane inst =
+  forAll (choose (5, 20)) $ \count ->
+  forAll genOnlineNode $ \node ->
+  let (nl, il, inst') = makeSmallEmptyCluster node count inst
+  in case Cluster.genAllocNodes defGroupList nl 2 True >>=
+     Cluster.tryAlloc nl il inst' of
+       Types.Bad _ -> False
+       Types.Ok as ->
+         case Cluster.asSolution as of
+           Nothing -> False
+           Just (xnl, xi, _, cv) ->
+             let il' = Container.add (Instance.idx xi) xi il
+                 tbl = Cluster.Table xnl il' cv []
+             in not (canBalance tbl True True False)

 
 -- | Checks that on a 2-5 node cluster, we can allocate a random
 -- instance spec via tiered allocation (whatever the original instance
 
 -- | Checks that on a 2-5 node cluster, we can allocate a random
 -- instance spec via tiered allocation (whatever the original instance

--- spec), on either one or two nodes
-prop_ClusterCanTieredAlloc node inst =
-    forAll (choose (2, 5)) $ \count ->
-    forAll (choose (1, 2)) $ \rqnodes ->
-    not (Node.offline node)
-            && not (Node.failN1 node)
-            && isNodeBig node 4
-            ==>
-    let nl = makeSmallCluster node count
-        il = Container.empty
-        allocnodes = Cluster.genAllocNodes defGroupList nl rqnodes True
-    in case allocnodes >>= \allocnodes' ->
-        Cluster.tieredAlloc nl il inst allocnodes' [] [] of
-         Types.Bad _ -> False
-         Types.Ok (_, _, il', ixes, cstats) -> not (null ixes) &&
-                                      IntMap.size il' == length ixes &&
-                                      length ixes == length cstats
+-- spec), on either one or two nodes.
+prop_ClusterCanTieredAlloc inst =
+  forAll (choose (2, 5)) $ \count ->
+  forAll (choose (1, 2)) $ \rqnodes ->
+  forAll (genOnlineNode `suchThat` (isNodeBig 4)) $ \node ->
+  let nl = makeSmallCluster node count
+      il = Container.empty
+      allocnodes = Cluster.genAllocNodes defGroupList nl rqnodes True
+  in case allocnodes >>= \allocnodes' ->
+    Cluster.tieredAlloc nl il (Just 1) inst allocnodes' [] [] of
+       Types.Bad _ -> False
+       Types.Ok (_, _, il', ixes, cstats) -> not (null ixes) &&
+                                             IntMap.size il' == length ixes &&
+                                             length ixes == length cstats

 
 -- | Checks that on a 4-8 node cluster, once we allocate an instance,
 
 -- | Checks that on a 4-8 node cluster, once we allocate an instance,

--- we can also evacuate it
-prop_ClusterAllocEvac node inst =
-    forAll (choose (4, 8)) $ \count ->
-    not (Node.offline node)
-            && not (Node.failN1 node)
-            && isNodeBig node 4
-            ==>
-    let nl = makeSmallCluster node count
-        il = Container.empty
-        inst' = setInstanceSmallerThanNode node inst
-    in case Cluster.genAllocNodes defGroupList nl 2 True >>=
-       Cluster.tryAlloc nl il inst' of
-         Types.Bad _ -> False
-         Types.Ok as ->
-             case Cluster.asSolutions as of
-               [] -> False
-               (xnl, xi, _, _):[] ->
-                   let sdx = Instance.sNode xi
-                       il' = Container.add (Instance.idx xi) xi il
-                   in case Cluster.tryEvac xnl il' [Instance.idx xi] [sdx] of
-                        Just _ -> True
-                        _ -> False
-               _ -> False
+-- we can also evacuate it.
+prop_ClusterAllocEvac inst =
+  forAll (choose (4, 8)) $ \count ->
+  forAll (genOnlineNode `suchThat` (isNodeBig 4)) $ \node ->
+  let (nl, il, inst') = makeSmallEmptyCluster node count inst
+  in case Cluster.genAllocNodes defGroupList nl 2 True >>=
+     Cluster.tryAlloc nl il inst' of
+       Types.Bad _ -> False
+       Types.Ok as ->
+         case Cluster.asSolution as of
+           Nothing -> False
+           Just (xnl, xi, _, _) ->
+             let sdx = Instance.sNode xi
+                 il' = Container.add (Instance.idx xi) xi il
+             in case IAlloc.processRelocate defGroupList xnl il'
+                  (Instance.idx xi) 1 [sdx] of
+                  Types.Ok _ -> True
+                  _ -> False

 
 -- | Check that allocating multiple instances on a cluster, then
 
 -- | Check that allocating multiple instances on a cluster, then

--- adding an empty node, results in a valid rebalance
-prop_ClusterAllocBalance node =
-    forAll (choose (3, 5)) $ \count ->
-    not (Node.offline node)
-            && not (Node.failN1 node)
-            && isNodeBig node 4
-            && not (isNodeBig node 8)
-            ==>
-    let nl = makeSmallCluster node count
-        (hnode, nl') = IntMap.deleteFindMax nl
-        il = Container.empty
-        allocnodes = Cluster.genAllocNodes defGroupList nl' 2 True
-        i_templ = createInstance Types.unitMem Types.unitDsk Types.unitCpu
-    in case allocnodes >>= \allocnodes' ->
-        Cluster.iterateAlloc nl' il i_templ allocnodes' [] [] of
-         Types.Bad _ -> False
-         Types.Ok (_, xnl, il', _, _) ->
-                   let ynl = Container.add (Node.idx hnode) hnode xnl
-                       cv = Cluster.compCV ynl
-                       tbl = Cluster.Table ynl il' cv []
-                   in canBalance tbl True True False
-
--- | Checks consistency
+-- adding an empty node, results in a valid rebalance.
+prop_ClusterAllocBalance =
+  forAll (genNode (Just 5) (Just 128)) $ \node ->
+  forAll (choose (3, 5)) $ \count ->
+  not (Node.offline node) && not (Node.failN1 node) ==>
+  let nl = makeSmallCluster node count
+      (hnode, nl') = IntMap.deleteFindMax nl
+      il = Container.empty
+      allocnodes = Cluster.genAllocNodes defGroupList nl' 2 True
+      i_templ = createInstance Types.unitMem Types.unitDsk Types.unitCpu
+  in case allocnodes >>= \allocnodes' ->
+    Cluster.iterateAlloc nl' il (Just 5) i_templ allocnodes' [] [] of
+       Types.Bad msg -> failTest $ "Failed to allocate: " ++ msg
+       Types.Ok (_, _, _, [], _) -> failTest "Failed to allocate: no instances"
+       Types.Ok (_, xnl, il', _, _) ->
+         let ynl = Container.add (Node.idx hnode) hnode xnl
+             cv = Cluster.compCV ynl
+             tbl = Cluster.Table ynl il' cv []
+         in printTestCase "Failed to rebalance" $
+            canBalance tbl True True False
+
+-- | Checks consistency.

 prop_ClusterCheckConsistency node inst =
   let nl = makeSmallCluster node 3
       [node1, node2, node3] = Container.elems nl
 prop_ClusterCheckConsistency node inst =
   let nl = makeSmallCluster node 3
       [node1, node2, node3] = Container.elems nl

@@ -831,7 +1116,7 @@ prop_ClusterCheckConsistency node inst =
      null (ccheck [(0, inst2)]) &&
      (not . null $ ccheck [(0, inst3)])
 
      null (ccheck [(0, inst2)]) &&
      (not . null $ ccheck [(0, inst3)])
 

--- For now, we only test that we don't lose instances during the split
+-- | For now, we only test that we don't lose instances during the split.

 prop_ClusterSplitCluster node inst =
   forAll (choose (0, 100)) $ \icnt ->
   let nl = makeSmallCluster node 2
 prop_ClusterSplitCluster node inst =
   forAll (choose (0, 100)) $ \icnt ->
   let nl = makeSmallCluster node 2

@@ -842,56 +1127,88 @@ prop_ClusterSplitCluster node inst =
      all (\(guuid, (nl'', _)) -> all ((== guuid) . Node.group)
                                  (Container.elems nl'')) gni
 
      all (\(guuid, (nl'', _)) -> all ((== guuid) . Node.group)
                                  (Container.elems nl'')) gni
 

-testCluster =
-    [ run prop_Score_Zero
-    , run prop_CStats_sane
-    , run prop_ClusterAlloc_sane
-    , run prop_ClusterCanTieredAlloc
-    , run prop_ClusterAllocEvac
-    , run prop_ClusterAllocBalance
-    , run prop_ClusterCheckConsistency
-    , run prop_ClusterSplitCluster
-    ]
-
--- | Check that opcode serialization is idempotent
-
+-- | Helper function to check if we can allocate an instance on a
+-- given node list.
+canAllocOn :: Node.List -> Int -> Instance.Instance -> Bool
+canAllocOn nl reqnodes inst =
+  case Cluster.genAllocNodes defGroupList nl reqnodes True >>=
+       Cluster.tryAlloc nl (Container.empty) inst of
+       Types.Bad _ -> False
+       Types.Ok as ->
+         case Cluster.asSolution as of
+           Nothing -> False
+           Just _ -> True
+
+-- | Checks that allocation obeys minimum and maximum instance
+-- policies. The unittest generates a random node, duplicates it count
+-- times, and generates a random instance that can be allocated on
+-- this mini-cluster; it then checks that after applying a policy that
+-- the instance doesn't fits, the allocation fails.
+prop_ClusterAllocPolicy node =
+  -- rqn is the required nodes (1 or 2)
+  forAll (choose (1, 2)) $ \rqn ->
+  forAll (choose (5, 20)) $ \count ->
+  forAll (arbitrary `suchThat` (canAllocOn (makeSmallCluster node count) rqn))
+         $ \inst ->
+  forAll (arbitrary `suchThat` (isFailure .
+                                Instance.instMatchesPolicy inst)) $ \ipol ->
+  let node' = Node.setPolicy ipol node
+      nl = makeSmallCluster node' count
+  in not $ canAllocOn nl rqn inst
+
+testSuite "Cluster"
+            [ 'prop_Score_Zero
+            , 'prop_CStats_sane
+            , 'prop_ClusterAlloc_sane
+            , 'prop_ClusterCanTieredAlloc
+            , 'prop_ClusterAllocEvac
+            , 'prop_ClusterAllocBalance
+            , 'prop_ClusterCheckConsistency
+            , 'prop_ClusterSplitCluster
+            , 'prop_ClusterAllocPolicy
+            ]
+
+-- ** OpCodes tests
+
+-- | Check that opcode serialization is idempotent.

 prop_OpCodes_serialization op =
   case J.readJSON (J.showJSON op) of
 prop_OpCodes_serialization op =
   case J.readJSON (J.showJSON op) of

-    J.Error _ -> False
-    J.Ok op' -> op == op'
+    J.Error e -> failTest $ "Cannot deserialise: " ++ e
+    J.Ok op' -> op ==? op'

   where _types = op::OpCodes.OpCode
 
   where _types = op::OpCodes.OpCode
 

-testOpCodes =
-  [ run prop_OpCodes_serialization
-  ]
+testSuite "OpCodes"
+            [ 'prop_OpCodes_serialization ]

 
 

--- | Check that (queued) job\/opcode status serialization is idempotent
+-- ** Jobs tests
+
+-- | Check that (queued) job\/opcode status serialization is idempotent.

 prop_OpStatus_serialization os =
   case J.readJSON (J.showJSON os) of
 prop_OpStatus_serialization os =
   case J.readJSON (J.showJSON os) of

-    J.Error _ -> False
-    J.Ok os' -> os == os'
+    J.Error e -> failTest $ "Cannot deserialise: " ++ e
+    J.Ok os' -> os ==? os'

   where _types = os::Jobs.OpStatus
 
 prop_JobStatus_serialization js =
   case J.readJSON (J.showJSON js) of
   where _types = os::Jobs.OpStatus
 
 prop_JobStatus_serialization js =
   case J.readJSON (J.showJSON js) of

-    J.Error _ -> False
-    J.Ok js' -> js == js'
+    J.Error e -> failTest $ "Cannot deserialise: " ++ e
+    J.Ok js' -> js ==? js'

   where _types = js::Jobs.JobStatus
 
   where _types = js::Jobs.JobStatus
 

-testJobs =
-  [ run prop_OpStatus_serialization
-  , run prop_JobStatus_serialization
-  ]
+testSuite "Jobs"
+            [ 'prop_OpStatus_serialization
+            , 'prop_JobStatus_serialization
+            ]

 
 

--- | Loader tests
+-- ** Loader tests

 
 prop_Loader_lookupNode ktn inst node =
 
 prop_Loader_lookupNode ktn inst node =

-  Loader.lookupNode nl inst node == Data.Map.lookup node nl
-  where nl = Data.Map.fromList ktn
+  Loader.lookupNode nl inst node ==? Data.Map.lookup node nl
+    where nl = Data.Map.fromList ktn

 
 prop_Loader_lookupInstance kti inst =
 
 prop_Loader_lookupInstance kti inst =

-  Loader.lookupInstance il inst == Data.Map.lookup inst il
-  where il = Data.Map.fromList kti
+  Loader.lookupInstance il inst ==? Data.Map.lookup inst il
+    where il = Data.Map.fromList kti

 
 prop_Loader_assignIndices nodes =
   Data.Map.size nassoc == length nodes &&
 
 prop_Loader_assignIndices nodes =
   Data.Map.size nassoc == length nodes &&

@@ -899,25 +1216,99 @@ prop_Loader_assignIndices nodes =
   (if not (null nodes)
    then maximum (IntMap.keys kt) == length nodes - 1
    else True)
   (if not (null nodes)
    then maximum (IntMap.keys kt) == length nodes - 1
    else True)

-  where (nassoc, kt) = Loader.assignIndices (map (\n -> (Node.name n, n)) nodes)
-
+    where (nassoc, kt) =
+            Loader.assignIndices (map (\n -> (Node.name n, n)) nodes)

 
 -- | Checks that the number of primary instances recorded on the nodes
 
 -- | Checks that the number of primary instances recorded on the nodes

--- is zero
+-- is zero.

 prop_Loader_mergeData ns =
   let na = Container.fromList $ map (\n -> (Node.idx n, n)) ns
   in case Loader.mergeData [] [] [] []
          (Loader.emptyCluster {Loader.cdNodes = na}) of
     Types.Bad _ -> False
 prop_Loader_mergeData ns =
   let na = Container.fromList $ map (\n -> (Node.idx n, n)) ns
   in case Loader.mergeData [] [] [] []
          (Loader.emptyCluster {Loader.cdNodes = na}) of
     Types.Bad _ -> False

-    Types.Ok (Loader.ClusterData _ nl il _) ->
+    Types.Ok (Loader.ClusterData _ nl il _ _) ->

       let nodes = Container.elems nl
           instances = Container.elems il
       in (sum . map (length . Node.pList)) nodes == 0 &&
          null instances
 
       let nodes = Container.elems nl
           instances = Container.elems il
       in (sum . map (length . Node.pList)) nodes == 0 &&
          null instances
 

-testLoader =
-  [ run prop_Loader_lookupNode
-  , run prop_Loader_lookupInstance
-  , run prop_Loader_assignIndices
-  , run prop_Loader_mergeData
-  ]
+-- | Check that compareNameComponent on equal strings works.
+prop_Loader_compareNameComponent_equal :: String -> Bool
+prop_Loader_compareNameComponent_equal s =
+  Loader.compareNameComponent s s ==
+    Loader.LookupResult Loader.ExactMatch s
+
+-- | Check that compareNameComponent on prefix strings works.
+prop_Loader_compareNameComponent_prefix :: NonEmptyList Char -> String -> Bool
+prop_Loader_compareNameComponent_prefix (NonEmpty s1) s2 =
+  Loader.compareNameComponent (s1 ++ "." ++ s2) s1 ==
+    Loader.LookupResult Loader.PartialMatch s1
+
+testSuite "Loader"
+            [ 'prop_Loader_lookupNode
+            , 'prop_Loader_lookupInstance
+            , 'prop_Loader_assignIndices
+            , 'prop_Loader_mergeData
+            , 'prop_Loader_compareNameComponent_equal
+            , 'prop_Loader_compareNameComponent_prefix
+            ]
+
+-- ** Types tests
+
+prop_Types_AllocPolicy_serialisation apol =
+  case J.readJSON (J.showJSON apol) of
+    J.Ok p -> p ==? apol
+    J.Error s -> failTest $ "Failed to deserialise: " ++ s
+      where _types = apol::Types.AllocPolicy
+
+prop_Types_DiskTemplate_serialisation dt =
+  case J.readJSON (J.showJSON dt) of
+    J.Ok p -> p ==? dt
+    J.Error s -> failTest $ "Failed to deserialise: " ++ s
+      where _types = dt::Types.DiskTemplate
+
+prop_Types_ISpec_serialisation ispec =
+  case J.readJSON (J.showJSON ispec) of
+    J.Ok p -> p ==? ispec
+    J.Error s -> failTest $ "Failed to deserialise: " ++ s
+      where _types = ispec::Types.ISpec
+
+prop_Types_IPolicy_serialisation ipol =
+  case J.readJSON (J.showJSON ipol) of
+    J.Ok p -> p ==? ipol
+    J.Error s -> failTest $ "Failed to deserialise: " ++ s
+      where _types = ipol::Types.IPolicy
+
+prop_Types_EvacMode_serialisation em =
+  case J.readJSON (J.showJSON em) of
+    J.Ok p -> p ==? em
+    J.Error s -> failTest $ "Failed to deserialise: " ++ s
+      where _types = em::Types.EvacMode
+
+prop_Types_opToResult op =
+  case op of
+    Types.OpFail _ -> Types.isBad r
+    Types.OpGood v -> case r of
+                        Types.Bad _ -> False
+                        Types.Ok v' -> v == v'
+  where r = Types.opToResult op
+        _types = op::Types.OpResult Int
+
+prop_Types_eitherToResult ei =
+  case ei of
+    Left _ -> Types.isBad r
+    Right v -> case r of
+                 Types.Bad _ -> False
+                 Types.Ok v' -> v == v'
+    where r = Types.eitherToResult ei
+          _types = ei::Either String Int
+
+testSuite "Types"
+            [ 'prop_Types_AllocPolicy_serialisation
+            , 'prop_Types_DiskTemplate_serialisation
+            , 'prop_Types_ISpec_serialisation
+            , 'prop_Types_IPolicy_serialisation
+            , 'prop_Types_EvacMode_serialisation
+            , 'prop_Types_opToResult
+            , 'prop_Types_eitherToResult
+            ]