Hi Iustin,
> One general comment: this doubles the number of calls (3→6) and also
> adds them in cluster modify (so 3→12).
Cluster modify was already tested, so it's 6→12, but of course your
point is still valid.
> Given that each separate call
> does create a job, submit it, etc., this will increase the QA time
> somewhat needlessly (all these could be unittests instead). I.e. it's
> fine to run one or two parameters, but you don't need to cmd-line QA all
> of them.
Will remove the 3 additional tests. (And will update the commit
message in case of LGTM)
> Anyway, that aside:
>> +# data for testing failures due to bad keys/values for disk parameters
>> +_FAIL_PARAMS = ("nonexistent:resync-rate=1",
>> + "drbd:nonexistent=1",
>> + "drbd:resync-rate=invalid",
>> + "drbd:data-stripes=invalid",
>> + "drbd:meta-stripes=invalid",
>> + "plain:stripes=invalid",
>> + )
>
> You're using tuples as list, please don't do that. If you iterate over a
> tuple it means you're misusing it.
I'll change it to a list, even if I don't really see why this is a
misuse of tuples — except for breaking the Ganeti conventions.
Thanks,
Andrea
Interdiff:
diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
index f746d5e..6624600 100644
--- a/qa/qa_cluster.py
+++ b/qa/qa_cluster.py
@@ -59,13 +59,10 @@ def _CheckFileOnAllNodes(filename, content):
# data for testing failures due to bad keys/values for disk parameters
-_FAIL_PARAMS = ("nonexistent:resync-rate=1",
+_FAIL_PARAMS = ["nonexistent:resync-rate=1",
"drbd:nonexistent=1",
"drbd:resync-rate=invalid",
- "drbd:data-stripes=invalid",
- "drbd:meta-stripes=invalid",
- "plain:stripes=invalid",
- )
+ ]
def TestClusterInitDisk():
