On 6 February 2013 14:38, Michael Hanselmann <[email protected]> wrote:
> Use a sort key function instead of using a comparing function (“cmp=…”).
> The latter is not supported in Python 3 and using a sort key function is
> easier.
> ---
>  qa/qa_config.py                  | 22 +++++++++++-----------
>  test/py/qa.qa_config_unittest.py | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 11 deletions(-)

> diff --git a/test/py/qa.qa_config_unittest.py 
> b/test/py/qa.qa_config_unittest.py
> index b3e4b58..aae163a 100755
> --- a/test/py/qa.qa_config_unittest.py
> +++ b/test/py/qa.qa_config_unittest.py
> @@ -349,6 +349,36 @@ class TestQaConfig(unittest.TestCase):
>      self.assertRaises(qa_error.OutOfNodesError, qa_config.AcquireNode,
>                        exclude=acquired, _cfg=self.config)
>
> +  def testAcquireNodeOrder(self):
> +    # Mark all nodes as marked (master excluded)
> +    for node in self.config["nodes"]:
> +      if node != self.config.GetMasterNode():
> +        node.MarkAdded()
> +
> +    nodecount = len(self.config["nodes"])
> +
> +    for iterations in [0, 1, 3, 100, 127, 7964]:
> +      acquired = []
> +
> +      for _ in range(iterations):
> +        node = qa_config.AcquireNode(_cfg=self.config)
> +        self.assertTrue(node.use_count > 0)
> +        #self.assertTrue(node.use_count < iterations)
> +        #self.assertTrue(node.use_count <= ((iterations / (nodecount - 1 )) 
> + 1))

Why are these lines commented? I believe the second test should be "<=
(iterations + nodecount - 1) / nodecount" (and it would make the other
one redundant). Also, you may be more strict and use the counter in
the inner for loop to make the check "== (counter / nodecount + 1)"
(where "counter" is the inner counter).

Rest LGTM, thanks.
Bernardo

Reply via email to