1. 15 Oct, 2012 1 commit
    • Iustin Pop's avatar
      Cleanup HTools.Types/BasicTypes imports · 01e52493
      Iustin Pop authored
      
      
      Before we reorganised the source tree, the 'Result' type was exported
      from HTools/Types.hs. This changed during the reorg, but at that time
      we didn't change the exports; instead, we kept re-exporting it from
      the old module for compatibility.
      
      In light of future changes to the Result type, let's stop this
      re-export and cleanup the imports of the other modules.
      
      One test is slightly rewritten with explicit types declaration (part
      of the values already needed one, let's make it explicit).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarMichael Hanselmann <hansmi@google.com>
      01e52493
  2. 10 Oct, 2012 1 commit
  3. 08 Oct, 2012 2 commits
  4. 26 Sep, 2012 1 commit
  5. 07 Sep, 2012 4 commits
    • Iustin Pop's avatar
      Improve the `AllocPolicy' test · 9e679143
      Iustin Pop authored
      
      
      This test has a few deficiencies, which this patch addresses:
      
      - using arbitrary 1 or 2 node count for allocation is obsolete,
        nowadays we need to use a number appropriate for the instance's disk
        template (and we should remove that parameter…)
      - generating a random node is sub-optimal, since we could generate an
        offline node, and no instance will fit on a cluster composed of only
        offline nodes
      - generating arbitrary instances "such that" they can be allocated is
        an expensive test; let's rather generate instances smaller than our
        template node, and add a check that they indeed can be allocated
      - using boolean return type, instead of nicely annotated properties
      
      For the nice annotation and the extra check, we need to change a
      helper function's signature, so that we can extract a bit more
      information out of it.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      9e679143
    • Iustin Pop's avatar
      Improve the `CanTieredAlloc' test · fb243105
      Iustin Pop authored
      
      
      Currently, this test is very slow. Upon investigation, this is due to
      how `tieredAlloc' works:
      
      - tries to allocate one instance
      - if failed, shrink the instance by the "most failed" resource
      - restart
      
      In this algorithm, if the "most failed" resource is e.g. memory, and
      the maximum available memory is much smaller than the current
      template, it means we will have to shrink and try to allocate many
      many times until we finally get with the to-be-allocated instance
      memory size to a reasonable value. In the real world, this is not the
      case, but when testing with arbitrary memory/node values, it can be
      that we execute the shrink hundreds of thousands of times per test.
      
      So we "improve" the test by directly generating an instance just
      slightly bigger than the node, so that we don't have to shrink too
      many times. This requires a new export from test/…/Instance.hs.
      
      Additionally, we allow up to 5 instances to be tiered-allocated, and
      we cleanup the test checks, making the conditions much, much more
      readable (IMHO).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      fb243105
    • Iustin Pop's avatar
      Add new test for checking multi-allocations · d83903ee
      Iustin Pop authored
      
      
      This test expands the "single-alloc-no-rebalance" by allocating a few
      instances on a small cluster, and ensuring that after we allocate all
      of them, either we can't rebalance or if we rebalance the score
      improvement is very small.
      
      The last condition is needed because sometime rounding errors (we're
      using double-precision floating point) can accumulate and result in
      what is a no real change in the cluster state, but with an
      infinitesimal score decrease (e.g. 1e-14).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      d83903ee
    • Iustin Pop's avatar
      Improve the prop_Alloc_sane test to detect mis-allocations · 650e5aa4
      Iustin Pop authored
      
      
      Currently, this just checks that a cluster cannot be rebalanced after
      a single instance allocation. However, we can also test whether the
      allocation decision computed a correct new cluster score, by checking
      that against the one computed from the actual new node list.
      
      Also, for nicer display, we convert the test from a Boolean to a
      Property, with nice annotations.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      650e5aa4
  6. 05 Sep, 2012 7 commits
    • Iustin Pop's avatar
      Further hlint fixes · 5b11f8db
      Iustin Pop authored
      Commit 2cdaf225, “Re-enable standard hlint warnings”, got it almost
      right. The only problem is that (confusingly) the default set of hints
      is not in HLint.Default, but in HLint.HLint (it includes Default and
      some built-ins).
      
      After changing the lint file to correctly include the defaults, we had
      another 128 suggestions:
      
        - Error: Eta reduce (2)
        - Error: Redundant bracket (4)
        - Error: Redundant do (17)
        - Error: Redundant lambda (7)
        - Error: Redundant return (1)
        - Warning: Avoid lambda (2)
        - Warning: Redundant $ (42)
        - Warning: Redundant bracket (35)
        - Warning: Use : (1)
        - Warning: Use String (4)
        - Warning: Use camelCase (10)
        - Warning: Use section (3)
      
      which are fixed by the current patch. Note that the 10 "Use camelCase"
      were all due to hlint not “knowing” the idiom of ‘case_’ (it does for
      ‘prop_’), for which I filled
      http://code.google.com/p/ndmitchell/issues/detail?id=558
      
      .
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      5b11f8db
    • Iustin Pop's avatar
      Rework CLI modules and tests · 51000365
      Iustin Pop authored
      While investigating how we could test the Daemon.hs module, I realised
      that we have a very, erm, sub-optimal situation:
      
      - HTools/CLI.hs has a nice IO/pure separation testing in cmdline
        parsing, which allows some basic functionality to be tested, but
        uses direct 'read' in many options, which fails at runtime when
        evaluating the argument, and not when parsing the options
      - Daemon.hs lacks that, but has a much nicer 'reqWithConversion'
        helper that can be used for nicer option parsing, and uses that +
        tryRead instead of plain 'read'
      
      Since this situation is very bad, let's clean it up. We introduce yet
      another module, Common.hs, that holds functionality common to all
      command line programs (daemons or not). We move the parsing to this
      module, and introduce a type class to handle option types which
      support --help/--version. This allows removal of duplicated code from
      CLI.hs and Daemon.hs.
      
      The other part of the patch is cleanup/rework of the tests for this
      code: we introduce some helpers (checkOpt, passFailOpt,
      checkEarlyExit) that can be used from the much-slimmer now tests for
      CLI and Daemon. In the common module, we just test the yes/no helper
      we have. Many new tests for boolean options and numeric options are
      added.
      
      A side change is the removal of the obsolete `--replay-count',
      `--test-size' options (unused since commit 95f6c931
      
      , “Switch Haskell
      test harness to test-framework”).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      51000365
    • Iustin Pop's avatar
      Add a small 'passTest' helper · 2e0bb81d
      Iustin Pop authored
      
      
      This is symmetric to failTest, and allows us to use it in cases where
      we need to return a property.
      
      While replacing 'property True' with it, I saw a case where we can
      simplify the use and thus reworked that check.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      2e0bb81d
    • Iustin Pop's avatar
      Replace manual arbitrary instances with genArbitrary · 7022db83
      Iustin Pop authored
      
      
      There are a few more that could be replaces, once we start using
      appropriate (new)types.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      7022db83
    • Iustin Pop's avatar
      Add a test helper for simple JSON serialisation testing · 63b068c1
      Iustin Pop authored
      
      
      While adding yet another JSON serialisation test, I realised that this
      can be trivially abstracted; hence this patch, replacing both simple
      versions (readJSON . showJSON == id) and the standard version (with
      different error messages) across the tests with a single function
      call.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      63b068c1
    • Iustin Pop's avatar
      Fixup test suite names · e09c1fa0
      Iustin Pop authored
      
      
      The names were not in a proper hierarchy, leading to inconsistencies
      about what they were actually tested.
      
      We change this by reproducing in the test names the relative hierarchy
      within the Ganeti directory, leading to nicer test suite names (in
      test-framework output).
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      e09c1fa0
    • Iustin Pop's avatar
      Simplify property and test case names · 20bc5360
      Iustin Pop authored
      
      
      Since we now have separate namespaces due to the multi-file split, we
      don't need to keep the name of the module in the property names, as we
      don't have so many potential conflicts anymore.
      
      We remove the group prefix handling from TestHelper and simply do a
      sed over all the test files, removing it from the function names.
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      20bc5360
  7. 04 Sep, 2012 2 commits