From 3582eef656f0026402385c68cbb6f35db483a02c Mon Sep 17 00:00:00 2001 From: Iustin Pop <iustin@google.com> Date: Wed, 12 Jan 2011 09:59:29 +0100 Subject: [PATCH] Run pylint over QA code too Right now, the QA code is not covered by pylint, and this shows at least one low-impact bug. This patch does the necessary changes to make QA pylint-clean, and the changes the makefile to run pylint for it. Notable changes: - qa_utils.GenericQueryTest: randfields was not used at all, and my belief is that it was indented to be used in order not to modify the input list; so I replaced randfields with fields, so we only shuffle the our local copy - qa_node.TestOutOfBand was using it's own copy of AcquireNode(), so I replaced it with the existing version - qa_os: was using 'dir' in a couple of places, replaced with dirname Signed-off-by: Iustin Pop <iustin@google.com> Reviewed-by: Michael Hanselmann <hansmi@google.com> --- Makefile.am | 3 +++ qa/ganeti-qa.py | 5 ++++- qa/qa_config.py | 4 ++-- qa/qa_error.py | 4 ++++ qa/qa_group.py | 6 +++++- qa/qa_instance.py | 7 ++++++- qa/qa_node.py | 13 ++++++------- qa/qa_os.py | 20 ++++++++++---------- qa/qa_rapi.py | 14 +++++++------- qa/qa_utils.py | 4 +++- 10 files changed, 50 insertions(+), 30 deletions(-) diff --git a/Makefile.am b/Makefile.am index 458545f6b..75ebe0816 100644 --- a/Makefile.am +++ b/Makefile.am @@ -802,6 +802,9 @@ check-local: check-dirs lint: $(BUILT_SOURCES) @test -n "$(PYLINT)" || { echo 'pylint' not found during configure; exit 1; } $(PYLINT) $(LINT_OPTS) $(lint_python_code) + cd $(top_srcdir)/qa && \ + PYTHONPATH=$(abs_top_srcdir) $(PYLINT) $(LINT_OPTS) \ + --rcfile ../pylintrc $(patsubst qa/%.py,%,$(qa_scripts)) # a dist hook rule for updating the vcs-version file; this is # hardcoded due to where it needs to build the file... diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py index 6dc909dcf..8e1934165 100755 --- a/qa/ganeti-qa.py +++ b/qa/ganeti-qa.py @@ -23,6 +23,9 @@ """ +# pylint: disable-msg=C0103 +# due to invalid name + import sys import datetime import optparse @@ -43,7 +46,7 @@ from ganeti import utils from ganeti import rapi from ganeti import constants -import ganeti.rapi.client +import ganeti.rapi.client # pylint: disable-msg=W0611 def _FormatHeader(line, end=72): diff --git a/qa/qa_config.py b/qa/qa_config.py index e02300804..62b0a0778 100644 --- a/qa/qa_config.py +++ b/qa/qa_config.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2007 Google Inc. +# Copyright (C) 2007, 2011 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 @@ -39,7 +39,7 @@ def Load(path): """Loads the passed configuration file. """ - global cfg + global cfg # pylint: disable-msg=W0603 cfg = serializer.LoadJson(utils.ReadFile(path)) diff --git a/qa/qa_error.py b/qa/qa_error.py index eb9131cb3..131b69bc8 100644 --- a/qa/qa_error.py +++ b/qa/qa_error.py @@ -19,6 +19,10 @@ # 02110-1301, USA. +"""Error definitions for QA. + +""" + class Error(Exception): """An error occurred during Q&A testing. diff --git a/qa/qa_group.py b/qa/qa_group.py index 38a9d7a09..290f837a1 100644 --- a/qa/qa_group.py +++ b/qa/qa_group.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2010 Google Inc. +# Copyright (C) 2010, 2011 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 @@ -19,6 +19,10 @@ # 02110-1301, USA. +"""QA tests for node groups. + +""" + from ganeti import constants from ganeti import query from ganeti import utils diff --git a/qa/qa_instance.py b/qa/qa_instance.py index cb598de9b..249a567e4 100644 --- a/qa/qa_instance.py +++ b/qa/qa_instance.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2007 Google Inc. +# Copyright (C) 2007, 2011 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 @@ -225,6 +225,9 @@ def TestInstanceConsole(instance): def TestReplaceDisks(instance, pnode, snode, othernode): """gnt-instance replace-disks""" + # pylint: disable-msg=W0613 + # due to unused pnode arg + # FIXME: should be removed from the function completely def buildcmd(args): cmd = ['gnt-instance', 'replace-disks'] cmd.extend(args) @@ -393,6 +396,8 @@ def _TestInstanceDiskFailure(instance, node, node2, onmaster): def TestInstanceMasterDiskFailure(instance, node, node2): """Testing disk failure on master node.""" + # pylint: disable-msg=W0613 + # due to unused args print qa_utils.FormatError("Disk failure on primary node cannot be" " tested due to potential crashes.") # The following can cause crashes, thus it's disabled until fixed diff --git a/qa/qa_node.py b/qa/qa_node.py index d35b7331c..187a8cfeb 100644 --- a/qa/qa_node.py +++ b/qa/qa_node.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2007 Google Inc. +# Copyright (C) 2007, 2011 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 @@ -19,6 +19,10 @@ # 02110-1301, USA. +"""Node-related QA tests. + +""" + from ganeti import utils from ganeti import constants from ganeti import query @@ -232,12 +236,7 @@ def TestOutOfBand(): """gnt-node power""" master = qa_config.GetMasterNode() - # Find first non master node for tests - for node in qa_config.get("nodes"): - if node != master: - break - else: - raise qa_error.Error("Can't find non-master node") + node = qa_config.AcquireNode(exclude=master) node_name = node["primary"] full_node_name = qa_utils.ResolveNodeName(node) diff --git a/qa/qa_os.py b/qa/qa_os.py index 566905860..2b6423df8 100644 --- a/qa/qa_os.py +++ b/qa/qa_os.py @@ -1,7 +1,7 @@ # # -# Copyright (C) 2007, 2008, 2009, 2010 Google Inc. +# Copyright (C) 2007, 2008, 2009, 2010, 2011 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 @@ -76,14 +76,14 @@ def _TestOsStates(): AssertCommand(new_cmd) -def _SetupTempOs(node, dir, valid): +def _SetupTempOs(node, dirname, valid): """Creates a temporary OS definition on the given node. """ sq = utils.ShellQuoteArgs - parts = [sq(["rm", "-rf", dir]), - sq(["mkdir", "-p", dir]), - sq(["cd", dir]), + parts = [sq(["rm", "-rf", dirname]), + sq(["mkdir", "-p", dirname]), + sq(["cd", dirname]), sq(["ln", "-fs", "/bin/true", "export"]), sq(["ln", "-fs", "/bin/true", "import"]), sq(["ln", "-fs", "/bin/true", "rename"])] @@ -103,18 +103,18 @@ def _SetupTempOs(node, dir, valid): AssertCommand(cmd, node=node) -def _RemoveTempOs(node, dir): +def _RemoveTempOs(node, dirname): """Removes a temporary OS definition. """ - AssertCommand(["rm", "-rf", dir], node=node) + AssertCommand(["rm", "-rf", dirname], node=node) def _TestOs(mode): """Generic function for OS definition testing """ - dir = _TEMP_OS_PATH + dirname = _TEMP_OS_PATH nodes = [] try: @@ -126,12 +126,12 @@ def _TestOs(mode): valid = True else: valid = bool(i % 2) - _SetupTempOs(node, dir, valid) + _SetupTempOs(node, dirname, valid) AssertCommand(["gnt-os", "diagnose"], fail=not mode==1) finally: for node in nodes: - _RemoveTempOs(node, dir) + _RemoveTempOs(node, dirname) def TestOsValid(): diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py index 2e19bf513..ece1b1a10 100644 --- a/qa/qa_rapi.py +++ b/qa/qa_rapi.py @@ -1,6 +1,6 @@ # -# Copyright (C) 2007, 2008, 2009, 2010 Google Inc. +# Copyright (C) 2007, 2008, 2009, 2010, 2011 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 @@ -27,19 +27,17 @@ import tempfile from ganeti import utils from ganeti import constants from ganeti import errors -from ganeti import serializer from ganeti import cli from ganeti import rapi -import ganeti.rapi.client +import ganeti.rapi.client # pylint: disable-msg=W0611 import ganeti.rapi.client_utils import qa_config import qa_utils import qa_error -from qa_utils import (AssertEqual, AssertNotEqual, AssertIn, AssertMatch, - StartLocalCommand) +from qa_utils import (AssertEqual, AssertIn, AssertMatch, StartLocalCommand) _rapi_ca = None @@ -52,6 +50,8 @@ def Setup(username, password): """Configures the RAPI client. """ + # pylint: disable-msg=W0603 + # due to global usage global _rapi_ca global _rapi_client global _rapi_username @@ -117,6 +117,8 @@ def Enabled(): def _DoTests(uris): + # pylint: disable-msg=W0212 + # due to _SendRequest usage results = [] for uri, verify, method, body in uris: @@ -322,8 +324,6 @@ def _WaitForRapiJob(job_id): """Waits for a job to finish. """ - master = qa_config.GetMasterNode() - def _VerifyJob(data): AssertEqual(data["id"], job_id) for field in JOB_FIELDS: diff --git a/qa/qa_utils.py b/qa/qa_utils.py index 20856f9d4..a5cc2cd0f 100644 --- a/qa/qa_utils.py +++ b/qa/qa_utils.py @@ -50,6 +50,8 @@ def _SetupColours(): """Initializes the colour constants. """ + # pylint: disable-msg=W0603 + # due to global usage global _INFO_SEQ, _WARNING_SEQ, _ERROR_SEQ, _RESET_SEQ # Don't use colours if stdout isn't a terminal @@ -403,7 +405,7 @@ def GenericQueryTest(cmd, fields): """ rnd = random.Random(hash(cmd)) - randfields = list(fields) + fields = list(fields) rnd.shuffle(fields) # Test a number of field combinations -- GitLab