Skip to content
Snippets Groups Projects
  1. Sep 18, 2012
  2. Sep 17, 2012
  3. Sep 14, 2012
  4. Sep 13, 2012
  5. Sep 12, 2012
  6. Sep 11, 2012
  7. Sep 07, 2012
    • Iustin Pop's avatar
      Simplify a bit more the test harness · f842aecd
      Iustin Pop authored
      
      We can build the test groups directly in the `testSuite' helper,
      instead of doing it (much later) in the test harness.
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      f842aecd
    • Iustin Pop's avatar
      Remove the slow/fast tests functionality · 44be51aa
      Iustin Pop authored
      
      Since the recent commits improved the speed of the two "slow" test
      groups to regular test speed, we can remove this kludge and simplify
      significantly our test runner, yay!
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      44be51aa
    • Iustin Pop's avatar
      Fix arbitrary ConfigData object generation · c3a8e06d
      Iustin Pop authored
      
      The Cluster object, as it is defined right now, has many '[String]'
      members, which means that in a standard arbitrary generator these will
      become very big, which is the reason for the current slowness of the
      test 'Config_serialisation'.
      
      By resizing the generator to 8 (arbitrary chosen to limit the list
      length and the string sizes), and by reducing a bit the node count, we
      can make this test be as fast as the others (about 10x
      improvement). This means we can test more cases, for the same cost.
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
      c3a8e06d
    • 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
    • Iustin Pop's avatar
      Merge branch 'devel-2.6' into submit · 107102af
      Iustin Pop authored
      
      * devel-2.6:
        Fix bug in non-mirrored instance allocation
        Fix gnt-debug iallocator
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarAgata Murawska <agatamurawska@google.com>
      107102af
    • Iustin Pop's avatar
      Merge branch 'stable-2.6' into devel-2.6 · 99c7795a
      Iustin Pop authored
      
      * stable-2.6:
        Fix bug in non-mirrored instance allocation
        Fix gnt-debug iallocator
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarAgata Murawska <agatamurawska@google.com>
      99c7795a
    • Iustin Pop's avatar
      Fix bug in non-mirrored instance allocation · 14b5d45f
      Iustin Pop authored
      
      The function `allocateOnSingle' has a bug in the calculation of the
      cluster score used for deciding which of the many target nodes to use
      in placing the instance: it uses the original node list for the score
      calculation.
      
      Due to this, since the original node list is the same for all target
      nodes, it means that basically `allocateOnSingle' returns the same
      score, no matter the target node, and hence the choosing of the node
      is arbitrary, instead of being done on the basis of the algorithm.
      
      This has gone uncaught until reported because the unittests only test
      1 allocation at a time on an empty cluster, and do not check the
      consistency of the score. I'll send separate patches on the master
      branch for adding more checks to prevent this in the future.
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarAgata Murawska <agatamurawska@google.com>
      14b5d45f
  8. Sep 05, 2012
    • 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
      Fix deserialisation bug in ResultEntry · 3ce788db
      Iustin Pop authored
      
      Found via the newly added unit-tests, which test most of the
      serialisation code in Query/Language (except for QueryResult, for
      which we already tests both sub-components separately).
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      3ce788db
    • Iustin Pop's avatar
      Add query filter tests · 90171729
      Iustin Pop authored
      
      These tests are node specific only because we don't have other query
      types implemented yet, but what they actually test is the various
      filter types.
      
      The tests are trying to cover most filter functionality; missing for
      now is proper checking for ContainsFilter and TrueFilter, the rest
      should be more or less covered.
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      90171729
    • Iustin Pop's avatar
      Add some unittests for node queries · b9bdc10e
      Iustin Pop authored
      
      These new tests check that:
      
      - no known fields return unknown
      - any unknown field returns unknown
      - the type of the fields is consistent between the getters and the
        field definition
      - the length of each result row corresponds with the number of fields
        queried, and the length of the field definitions returned
      - the length of the rows corresponds to the number of nodes
      - querying fields on empty fields returns all fields
      
      Finally this patch found a bug, in that the pinst_list/sinst_list
      fields were declared as QFTNumber (copy-paste error from
      pinst_cnt/sinst_cnt), yay!
      
      I also changed genEmptyCluster to ensure that it generates unique node
      names, so that the number of result rows is consistent with what we
      requested, and switched ResultEntry from a normal constructor to
      record syntax, so that we can extract the fields without having to use
      pattern matching.
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      b9bdc10e
    • 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
      Add entire ConfigData serialisation tests · 9924d61e
      Iustin Pop authored
      
      Using the recently-added genArbitrary, we can now implement Arbitrary
      instances for even "huge" objects like Cluster, so let's use that to
      implement entire ConfigData serialisation tests.
      
      Note that, as we don't have yet proper types for some of the Params
      fields, we have to cheat via FlexibleInstances and
      TypeSynonymInstances, using either empty items or real arbitrary
      values.
      
      Signed-off-by: default avatarIustin Pop <iustin@google.com>
      Reviewed-by: default avatarRené Nussbaumer <rn@google.com>
      9924d61e
    • 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
Loading