This patch gets the tests down to 0 warnings and errors, in some cases by simply disabling warnings. Many of these warnings are good warnings and valuable, but not for a test suite were we want to do odd things to prod at the internals of functions and classes.
Signed-off-by: Dylan Baker <[email protected]> --- framework/tests/core_tests.py | 3 ++ framework/tests/dmesg_tests.py | 34 +++++++++++--------- framework/tests/exectest_test.py | 9 ++++-- framework/tests/gleantest_tests.py | 12 ++++--- framework/tests/glsl_parser_test_tests.py | 52 ++++++++++++++++++++----------- framework/tests/integration_tests.py | 3 +- framework/tests/log_tests.py | 6 ++-- framework/tests/profile_tests.py | 2 ++ framework/tests/results_tests.py | 7 ++++- framework/tests/results_v0_tests.py | 6 ++-- framework/tests/run_parser_tests.py | 2 ++ framework/tests/status_tests.py | 21 +++++++------ framework/tests/summary_tests.py | 5 ++- framework/tests/utils.py | 2 +- 14 files changed, 106 insertions(+), 58 deletions(-) diff --git a/framework/tests/core_tests.py b/framework/tests/core_tests.py index b44bb83..359396a 100644 --- a/framework/tests/core_tests.py +++ b/framework/tests/core_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=no-self-use + """ Module providing tests for the core module """ @@ -109,6 +111,7 @@ def test_parse_listfile_tilde(): class TestGetConfig(utils.TestWithEnvClean): + """ Tests for core.get_config() """ CONF_FILE = textwrap.dedent(""" [nose-test] ; a section for testing behavior diff --git a/framework/tests/dmesg_tests.py b/framework/tests/dmesg_tests.py index 4bd1bcf..b2df01d 100644 --- a/framework/tests/dmesg_tests.py +++ b/framework/tests/dmesg_tests.py @@ -18,6 +18,9 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=too-few-public-methods,no-self-use,invalid-name, +# pylint: disable=protected-access + """ Provides tests for the dmesg class Tests that require sudo have sudo in their name, if you don't have sudo or @@ -83,7 +86,7 @@ class DummyLog(object): def __init__(self): pass - def pre_log(self, *args): + def pre_log(self, *args): # pylint: disable=unused-argument return None def log(self, *args): @@ -95,6 +98,9 @@ class DummyLog(object): class TestDmesg(dmesg.BaseDmesg): """ A special implementation of Dmesg that is easy to test with """ + def __init__(self): + super(TestDmesg, self).__init__() + def update_dmesg(self): pass @@ -229,12 +235,12 @@ def test_update_result_replace(): result['subtest']['test'] = res return result - dmesg = TestDmesg() + dmesg_ = TestDmesg() for res in ['pass', 'fail', 'crash', 'warn', 'skip', 'notrun']: - dmesg.regex = None - dmesg._new_messages = ['add', 'some', 'stuff'] - new_result = dmesg.update_result(create_test_result(res)) + dmesg_.regex = None + dmesg_._new_messages = ['add', 'some', 'stuff'] + new_result = dmesg_.update_result(create_test_result(res)) check_update_result.description = "Test update_result: {0}".format(res) yield check_update_result, new_result['result'], res @@ -245,9 +251,9 @@ def test_update_result_replace(): # check that the status is not updated when Dmesg.regex is set and does # not match the dmesg output - dmesg.regex = re.compile("(?!)") - dmesg._new_messages = ['more', 'new', 'stuff'] - new_result = dmesg.update_result(create_test_result(res)) + dmesg_.regex = re.compile("(?!)") + dmesg_._new_messages = ['more', 'new', 'stuff'] + new_result = dmesg_.update_result(create_test_result(res)) check_equal_result.description = \ "Test update_result with non-matching regex: {0}".format(res) @@ -255,9 +261,9 @@ def test_update_result_replace(): # check that the status is updated when Dmesg.regex is set and matches # the dmesg output - dmesg.regex = re.compile("piglit.*test") - dmesg._new_messages = ['piglit.awesome.test', 'and', 'stuff'] - new_result = dmesg.update_result(create_test_result(res)) + dmesg_.regex = re.compile("piglit.*test") + dmesg_._new_messages = ['piglit.awesome.test', 'and', 'stuff'] + new_result = dmesg_.update_result(create_test_result(res)) check_update_result.description = \ "Test update_result with matching regex: {0} ".format(res) @@ -355,9 +361,9 @@ def check_classes_dmesg(test_class, test_args): test = test_class(test_args) test._test_hook_execute_run = _write_dev_kmesg - json = DummyJsonWriter() + json_ = DummyJsonWriter() - test.execute(None, DummyLog(), json, test) + test.execute(None, DummyLog(), json_, test) - nt.assert_in(json.result['result'], ['dmesg-warn', 'dmesg-fail'], + nt.assert_in(json_.result['result'], ['dmesg-warn', 'dmesg-fail'], msg="{0} did not update status with dmesg".format(type(test))) diff --git a/framework/tests/exectest_test.py b/framework/tests/exectest_test.py index 3a639d2..dfbef79 100644 --- a/framework/tests/exectest_test.py +++ b/framework/tests/exectest_test.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=invalid-name + """ Tests for the exectest module """ import nose.tools as nt @@ -61,7 +63,7 @@ def test_timeout(): """ Test that Test.timeout works correctly """ def helper(): - if (test.result['returncode'] == 0): + if test.result['returncode'] == 0: test.result['result'] = "pass" test = TestTest("/usr/bin/sleep 60") @@ -70,11 +72,12 @@ def test_timeout(): test.run() assert test.result['result'] == 'timeout' + def test_timeout_pass(): """ Test that the correct result is returned if a test does not timeout """ def helper(): - if (test.result['returncode'] == 0): + if test.result['returncode'] == 0: test.result['result'] = "pass" test = TestTest("/usr/bin/true") @@ -98,7 +101,7 @@ def test_piglittest_interpret_result_subtest(): test.result['out'] = ('PIGLIT: {"result": "pass"}\n' 'PIGLIT: {"subtest": {"subtest": "pass"}}\n') test.interpret_result() - nt.assert_equal(test.result['subtest']['subtest'],'pass') + nt.assert_equal(test.result['subtest']['subtest'], 'pass') def test_piglitest_no_clobber(): diff --git a/framework/tests/gleantest_tests.py b/framework/tests/gleantest_tests.py index d157b09..7dbe0f1 100644 --- a/framework/tests/gleantest_tests.py +++ b/framework/tests/gleantest_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=invalid-name + """ Tests for the glean class. Requires Nose """ from __future__ import print_function @@ -36,13 +38,13 @@ def test_GLOBAL_PARAMS_assignment(): """ Test to ensure that GleanTest.GLOBAL_PARAMS are correctly assigned Specifically this tests for a bug where GLOBAL_PARAMS only affected - instances of GleanTest created after GLOBAL_PARAMS were set, so changing the - GLOBAL_PARAMS value had unexpected results. + instances of GleanTest created after GLOBAL_PARAMS were set, so changing + the GLOBAL_PARAMS value had unexpected results. If this test passes the GleanTest.command attributes will be the same in - the instance created before the GLOBAL_PARAMS assignment and the one created - after. A failure means the that GLOBAL_PARAMS are not being added to tests - initialized before it is set. + the instance created before the GLOBAL_PARAMS assignment and the one + created after. A failure means the that GLOBAL_PARAMS are not being added + to tests initialized before it is set. """ test1 = GleanTest('basic') diff --git a/framework/tests/glsl_parser_test_tests.py b/framework/tests/glsl_parser_test_tests.py index cfaeaa3..7409e5c 100644 --- a/framework/tests/glsl_parser_test_tests.py +++ b/framework/tests/glsl_parser_test_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=invalid-name + """ Provides tests for the shader_test module """ import sys @@ -111,8 +113,9 @@ def test_cpp_comments(): '// [end config]\n') test, name = _check_config(content) - nt.assert_equal(test.command, [os.path.join(TEST_BIN_DIR, 'glslparsertest'), - name, 'pass', '1.00'], + nt.assert_equal(test.command, + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), name, + 'pass', '1.00'], msg="C++ style comments were not properly parsed") @@ -126,8 +129,9 @@ def test_c_comments(): ' */\n') test, name = _check_config(content) - nt.assert_equal(test.command, [os.path.join(TEST_BIN_DIR, 'glslparsertest'), - name, 'pass', '1.00'], + nt.assert_equal(test.command, + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), name, + 'pass', '1.00'], msg="C style comments were not properly parsed") @@ -141,8 +145,9 @@ def test_blank_in_config(): test, name = _check_config(content) - nt.assert_equal(test.command, [os.path.join(TEST_BIN_DIR, 'glslparsertest'), - name, 'pass', '1.00'], + nt.assert_equal(test.command, + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), name, + 'pass', '1.00'], msg="A newline in a C++ style comment was not properly " "parsed.") @@ -157,8 +162,9 @@ def test_empty_in_config(): test, name = _check_config(content) - nt.assert_equal(test.command, [os.path.join(TEST_BIN_DIR, 'glslparsertest'), - name, 'pass', '1.00'], + nt.assert_equal(test.command, + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), name, + 'pass', '1.00'], msg="A blank commented line in a C++ style comment was not" " properly parsed.") @@ -182,20 +188,28 @@ def check_config_to_command(config, result): def test_config_to_command(): """ Generate tests that confirm the config file is correctly parsed """ content = [ - ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n// [end config]\n', + ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n' + '// [end config]\n', [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00'], 'all required options'), - ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n//check_link: true\n// [end config]\n', - [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00', '--check-link'], + ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n' + '//check_link: true\n// [end config]\n', + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00', + '--check-link'], 'check_link true'), - ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n//check_link: false\n// [end config]\n', + ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n' + '//check_link: false\n// [end config]\n', [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00'], 'check_link false'), - ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n//require_extensions: ARB_foo\n// [end config]\n', - [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00', 'ARB_foo'], + ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n' + '// require_extensions: ARB_foo\n// [end config]\n', + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00', + 'ARB_foo'], 'one required_extension'), - ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n//require_extensions: ARB_foo ARB_bar\n// [end config]\n', - [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00', 'ARB_foo', 'ARB_bar'], + ('// [config]\n// expect_result: pass\n// glsl_version: 1.00\n' + '//require_extensions: ARB_foo ARB_bar\n// [end config]\n', + [os.path.join(TEST_BIN_DIR, 'glslparsertest'), 'pass', '1.00', + 'ARB_foo', 'ARB_bar'], 'multiple required_extensions'), ] @@ -217,7 +231,7 @@ def test_bad_section_name(): _, name = _check_config(content) nt.eq_(e.exception.message, - 'Key new_awesome_key in file {0 is not a valid key for a ' + 'Key new_awesome_key in file {0} is not a valid key for a ' 'glslparser test config block'.format(name)) @@ -286,8 +300,8 @@ def glslparser_exetensions_seperators(): """ problems = [ ('comma seperator', '// require_extensions: ARB_ham, ARB_turkey\n'), - ('semi-colon seperator', '// require_extensions: ARB_ham; ARB_turkey\n'), - ('trailing semi-colon', '// require_extensions: ARB_ham ARB_turkey\n;'), + ('semi-colon seperator', '// require_extensions: ARB_ham; ARB_pbj\n'), + ('trailing semi-colon', '// require_extensions: ARB_ham ARB_pbj\n;'), ('Non-alpha character', '// require_extensions: ARB_$$$\n'), ] diff --git a/framework/tests/integration_tests.py b/framework/tests/integration_tests.py index ca154c0..91c3d68 100644 --- a/framework/tests/integration_tests.py +++ b/framework/tests/integration_tests.py @@ -40,7 +40,8 @@ def _import(name): """ Helper for importing modules """ try: return importlib.import_module(name) - except (ConfigParser.NoOptionError, ConfigParser.NoSectionError, SystemExit): + except (ConfigParser.NoOptionError, ConfigParser.NoSectionError, + SystemExit): raise SkipTest('No config section for {}'.format(name)) diff --git a/framework/tests/log_tests.py b/framework/tests/log_tests.py index d66a1a4..587915a 100644 --- a/framework/tests/log_tests.py +++ b/framework/tests/log_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=protected-access + """ Module provides tests for log.py module """ from __future__ import print_function @@ -58,12 +60,12 @@ def test_quietlog_log_state_update(): log_inst.log('pass') # This lambda is necissary, without it description cannot be set + # pylint: disable=unnecessary-lambda check = lambda x, y: nt.assert_equal(x, y) for name, value in [('total', 100), ('summary', {'pass': 1}), ('complete', 1)]: check.description = \ - 'State value {0} is incremented by QuiteLog.log()'.format( - name, value) + 'State value {0} is incremented by QuiteLog.log()'.format(name) yield check, logger._state[name], value diff --git a/framework/tests/profile_tests.py b/framework/tests/profile_tests.py index 73593ad..cff2dc9 100644 --- a/framework/tests/profile_tests.py +++ b/framework/tests/profile_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=protected-access,invalid-name + """ Provides test for the framework.profile modules """ import copy diff --git a/framework/tests/results_tests.py b/framework/tests/results_tests.py index 449ccb2..36814de 100644 --- a/framework/tests/results_tests.py +++ b/framework/tests/results_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=invalid-name + """ Module providing tests for the core module """ @@ -73,7 +75,7 @@ def test_load_results_folder_as_main(): def test_load_results_folder(): - """ Test that load_results takes a folder with a file named results.json """ + """Test that load_results takes a folder with a file named results.json""" with utils.tempdir() as tdir: with open(os.path.join(tdir, 'results.json'), 'w') as tfile: tfile.write(json.dumps(utils.JSON_DATA)) @@ -175,6 +177,7 @@ def test_get_backend(): class TestJunitNoTests(utils.StaticDirectory): + """ Test Junit backend with no tests """ @classmethod def setup_class(cls): super(TestJunitNoTests, cls).setup_class() @@ -196,6 +199,7 @@ class TestJunitNoTests(utils.StaticDirectory): class TestJUnitSingleTest(TestJunitNoTests): + """ Test Junit backend with one test """ @classmethod def setup_class(cls): super(TestJUnitSingleTest, cls).setup_class() @@ -224,6 +228,7 @@ class TestJUnitSingleTest(TestJunitNoTests): class TestJUnitMultiTest(TestJUnitSingleTest): + """ Test Junit backend with more than one test """ @classmethod def setup_class(cls): super(TestJUnitMultiTest, cls).setup_class() diff --git a/framework/tests/results_v0_tests.py b/framework/tests/results_v0_tests.py index 2f465d9..c09ed82 100644 --- a/framework/tests/results_v0_tests.py +++ b/framework/tests/results_v0_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=protected-access,invalid-name + """ Module provides tests for converting version zero results to version 1 """ import json @@ -100,8 +102,8 @@ DATA['tests'].update({ }, }) -with utils.with_tempfile(json.dumps(DATA)) as f: - RESULT = results._update_zero_to_one(results.load_results(f)) +with utils.with_tempfile(json.dumps(DATA)) as _f: + RESULT = results._update_zero_to_one(results.load_results(_f)) def test_dmesg(): diff --git a/framework/tests/run_parser_tests.py b/framework/tests/run_parser_tests.py index d355bb2..36ad45d 100644 --- a/framework/tests/run_parser_tests.py +++ b/framework/tests/run_parser_tests.py @@ -18,6 +18,8 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE # SOFTWARE. +# pylint: disable=no-self-use,protected-access + """ Module of tests for the run commandline parser """ import sys diff --git a/framework/tests/status_tests.py b/framework/tests/status_tests.py index 81423b1..65d14eb 100644 --- a/framework/tests/status_tests.py +++ b/framework/tests/status_tests.py @@ -41,9 +41,9 @@ PROBLEMS = STATUSES[1:] # Create lists of fixes and regressions programmatically based on the STATUSES # list. This means less code, and easier expansion changes. REGRESSIONS = list(itertools.combinations(STATUSES, 2)) + \ - list(itertools.combinations(["skip"] + PROBLEMS, 2)) + list(itertools.combinations(["skip"] + PROBLEMS, 2)) FIXES = list(itertools.combinations(reversed(STATUSES), 2)) + \ - list(itertools.combinations(list(reversed(PROBLEMS)) + ["skip"], 2)) + list(itertools.combinations(list(reversed(PROBLEMS)) + ["skip"], 2)) # The statuses that don't cause changes when transitioning from one another NO_OPS = ('skip', 'notrun') @@ -61,7 +61,7 @@ def initialize_nochangestatus(): def compare_status_nochangestatus(): """ Status and NoChangeStatus can be compared with < """ - status.CRASH < status.PASS + nt.assert_less(status.CRASH, status.PASS) def check_lookup(stat): @@ -137,7 +137,7 @@ def test_is_change(): def check_not_change(new, old): - """ Check that a status doesn't count as a change + """ Check that a status doesn't count as a change This checks that new < old and old < new do not return true. This is meant for checking skip and notrun, which we don't want to show up as regressions @@ -191,7 +191,7 @@ def _max_stat_nochange(nochange, stat): msg="max({stat}, {nochange}) = {stat}".format(**locals())) -def check_operator(obj, op, result): +def check_operator(obj, op, result): # pylint: disable=invalid-name """ Test that the result of running an operator on an object is expected Arguments: @@ -203,6 +203,7 @@ def check_operator(obj, op, result): nt.assert_equal(op(obj), result) +# pylint: disable=invalid-name def check_operator_equal(obj, comp, op, result): """ Test that the result of running an operator on an object is expected @@ -215,6 +216,7 @@ def check_operator_equal(obj, comp, op, result): nt.assert_equal(op(obj, comp), result) +# pylint: disable=invalid-name def check_operator_not_equal(obj, comp, op, result): """ Test that the result of running an operator on an object is expected @@ -248,7 +250,8 @@ def test_nochangestatus_magic(): 'Operator ne works with type: {0} on class ' 'status.NoChangeStatus'.format(type_) ) - yield check_operator_not_equal, obj, comp, lambda x, y: x.__ne__(y), True + yield (check_operator_not_equal, obj, comp, lambda x, y: x.__ne__(y), + True) @utils.nose_generator @@ -292,16 +295,16 @@ def test_status_magic(): @nt.raises(TypeError) def test_status_eq_raises(): """ Comparing Status and an unlike object with eq raises a TypeError """ - status.PASS == dict() + status.PASS == dict() # pylint: disable=expression-not-assigned @nt.raises(TypeError) def test_nochangestatus_eq_raises(): """ NoChangeStatus == !(str, unicode, Status) raises TypeError """ - status.NOTRUN == dict() + status.NOTRUN == dict() # pylint: disable=expression-not-assigned @nt.raises(TypeError) def test_nochangestatus_ne_raises(): """ NoChangeStatus != (str, unicode, Status) raises TypeError """ - status.NOTRUN != dict() + status.NOTRUN != dict() # pylint: disable=expression-not-assigned diff --git a/framework/tests/summary_tests.py b/framework/tests/summary_tests.py index 9152866..986ebdd 100644 --- a/framework/tests/summary_tests.py +++ b/framework/tests/summary_tests.py @@ -57,7 +57,7 @@ def test_summary_add_to_set(): ('pass', 'timeout', 'regressions'), ('pass', 'timeout', 'problems')]: check_sets.description = "{0} -> {1} should be added to {2}".format( - ostat, nstat, set_) + ostat, nstat, set_) yield check_sets, old, ostat, new, nstat, set_ @@ -77,8 +77,10 @@ def check_sets(old, ostat, new, nstat, set_): class TestSubtestHandling(object): + """ test subtest handling in summary """ @classmethod def setup_class(cls): + """ Create a summary object to be shared """ data = copy.deepcopy(utils.JSON_DATA) data['tests']['with_subtests'] = {} data['tests']['with_subtests']['result'] = 'pass' @@ -100,6 +102,7 @@ class TestSubtestHandling(object): msg="Summary.fraction['fake-tests']['with_subtests'] should " "be (0, 3), but isn't") + # pylint: disable=invalid-name def test_tests_w_subtests_are_groups(self): """ summary.Summary: Tests with subtests should be a group diff --git a/framework/tests/utils.py b/framework/tests/utils.py index 9ea44e3..b4bcccf 100644 --- a/framework/tests/utils.py +++ b/framework/tests/utils.py @@ -112,7 +112,7 @@ def tempdir(): shutil.rmtree(tdir) [email protected] [email protected] # pylint: disable=too-few-public-methods class GeneratedTestWrapper(object): """ An object proxy for nose test instances -- 2.1.1 _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
