From 707b65fd3480518c0f414c6470e5589b382f2c1f Mon Sep 17 00:00:00 2001 From: Mustafa Kemal Gilor Date: Tue, 20 Aug 2024 11:48:18 +0300 Subject: [PATCH] project: add ruff as additional checker we're now also linting the test files too. made some changes to make test files compliant with the pylint settings. Closes SET-981 Signed-off-by: Mustafa Kemal Gilor --- .github/workflows/pipeline.yml | 5 ++ extras/py36-all-requirements.py | 1 - hotkdump/main.py | 4 +- pyproject.toml | 1 + tests/test_hotkdump.py | 41 +++++----------- tests/test_kdump_file_header.py | 86 +++++++++++++-------------------- tests/utils.py | 60 +++++++++++++++++------ tox.ini | 18 +++++-- 8 files changed, 114 insertions(+), 102 deletions(-) diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 392de4f..34cc692 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -70,6 +70,11 @@ jobs: if: matrix.toxenv== 'py312' run: | tox -e pylint + + - name: Run ruff + if: matrix.toxenv== 'py312' + run: | + tox -e ruff Test-Snap: runs-on: ubuntu-20.04 steps: diff --git a/extras/py36-all-requirements.py b/extras/py36-all-requirements.py index da55861..002d918 100644 --- a/extras/py36-all-requirements.py +++ b/extras/py36-all-requirements.py @@ -4,7 +4,6 @@ # setuptools do not support reading optional-dependencies # from pyproject.toml file. Drop this file when python 3.6 # support is dropped. -import sys import toml with open("pyproject.toml", "r") as f: diff --git a/hotkdump/main.py b/hotkdump/main.py index 17c3636..4b2d67d 100755 --- a/hotkdump/main.py +++ b/hotkdump/main.py @@ -21,8 +21,8 @@ # These are actually need to be run after the sys.path is updated, so # we're silencing the warning. # pylint: disable=wrong-import-position -from hotkdump.core.hotkdump import Hotkdump, HotkdumpParameters -from hotkdump.core.exceptions import NotAKernelCrashDumpException +from hotkdump.core.hotkdump import Hotkdump, HotkdumpParameters # noqa: E402 +from hotkdump.core.exceptions import NotAKernelCrashDumpException # noqa: E402 def main(): diff --git a/pyproject.toml b/pyproject.toml index 638806e..7ccf205 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ testing = [ "pytest>=7", "pytest-sugar", "pylint", + "ruff;python_version>'3.6'", # The below are ubuntu-dev-tools's dependencies but unfortunately # the requirements.txt file is not referred in setup.py so we have # to list them here diff --git a/tests/test_hotkdump.py b/tests/test_hotkdump.py index 0022eec..347a5c6 100644 --- a/tests/test_hotkdump.py +++ b/tests/test_hotkdump.py @@ -1,10 +1,9 @@ #!/usr/bin/env python3 -# Copyright 2023 Canonical Limited. -# SPDX-License-Identifier: GPL-3.0 +"""`hotkdump` class unit tests. -""" -`hotkdump` class unit tests. +Copyright 2023 Canonical Limited. +SPDX-License-Identifier: GPL-3.0 """ from unittest import mock, TestCase @@ -16,29 +15,13 @@ from tests.utils import ( assert_has_no_such_calls, - mock_file_ctx, - mock_stat_obj, - fill_zeros, + MockFileCtx, + MockStatObj, + MOCK_HDR ) mock.Mock.assert_has_no_such_calls = assert_has_no_such_calls -MOCK_HDR = fill_zeros( - b"KDUMP " # signature - + b"\x01\x02\x03\x04" # header_version - + fill_zeros(b"sys", 65) # system - + fill_zeros(b"node", 65) # node - + fill_zeros(b"release", 65) # release - + fill_zeros(b"#version-443", 65) # version - + fill_zeros(b"machine", 65) # machine - + fill_zeros(b"domain", 65) # domain - + b"\x02" * 6 # padding - + b"\x01\x00\x00\x00\x00\x00\x00\x00" # timestamp_sec - + b"\x02\x00\x00\x00\x00\x00\x00\x00" # timestamp_usec - + b"\x03\x00\x00\x00" # status - + b"\x00\x10\x00\x00", # block_size - 4096, -) + fill_zeros(b"", 4096) @mock.patch.multiple( @@ -52,7 +35,7 @@ "os.path", dirname=lambda x: x, realpath=lambda x: x, exists=lambda x: True ) @mock.patch.multiple("shutil", which=lambda x: x) -@mock.patch("builtins.open", mock_file_ctx(bytes=MOCK_HDR, name="name")) +@mock.patch("builtins.open", MockFileCtx(file_bytes=MOCK_HDR, name="name")) class HotkdumpTest(TestCase): """test hotkdump class public api""" @@ -109,7 +92,7 @@ def test_arch(self): uut.kdump_file.ddhdr.utsname.machine = "invalid" self.assertRaises(NotImplementedError, uut.get_architecture) - @mock.patch("builtins.open", mock_file_ctx(bytes=MOCK_HDR, name="name")) + @mock.patch("builtins.open", MockFileCtx(file_bytes=MOCK_HDR, name="name")) def test_kdump_hdr(self): """Test if the kdump_header has the correct values included in the MOCK_HDR after opening the fake vmcore file @@ -178,6 +161,7 @@ def test_write_crash_commands_file(self): params = HotkdumpParameters(dump_file_path="empty", output_file_path="hkd.test") uut = Hotkdump(params) uut.temp_working_dir.name = "/tmpdir" + # pylint: disable=line-too-long expected_output = textwrap.dedent( r""" !echo "---------------------------------------" >> hkd.test @@ -232,6 +216,7 @@ def test_write_crash_commands_file(self): !echo "" >> hkd.test """ ).strip() + # pylint: enable=line-too-long with mock.patch("builtins.open", new_callable=mock.mock_open()) as mo: contents = None @@ -493,7 +478,7 @@ def test_post_run_ddeb_age_policy(self): "file4.ddeb", ] - mock_stat.side_effect = lambda fname: mock_stat_obj( + mock_stat.side_effect = lambda fname: MockStatObj( fname, { "/path/to/ddebs/file1.ddeb": { @@ -552,7 +537,7 @@ def test_post_run_ddeb_total_size_policy(self): "file4.ddeb", ] - mock_stat.side_effect = lambda fname: mock_stat_obj( + mock_stat.side_effect = lambda fname: MockStatObj( fname, { "/path/to/ddebs/file1.ddeb": {"atime": 0, "size": 15000}, @@ -618,7 +603,7 @@ def test_post_run_ddeb_retention_disabled( "file3.txt", "file4.ddeb", ] - mock_stat.side_effect = lambda fname, *args, **kwargs: mock_stat_obj( + mock_stat.side_effect = lambda fname, *args, **kwargs: MockStatObj( fname, { "/path/to/ddebs/file1.ddeb": {"atime": 0, "size": 15000}, diff --git a/tests/test_kdump_file_header.py b/tests/test_kdump_file_header.py index c686e99..4122685 100644 --- a/tests/test_kdump_file_header.py +++ b/tests/test_kdump_file_header.py @@ -1,10 +1,9 @@ #!/usr/bin/env python3 -# Copyright 2023 Canonical Limited. -# SPDX-License-Identifier: GPL-3.0 +"""`hotkdump` class unit tests. -""" -`hotkdump` class unit tests. +Copyright 2023 Canonical Limited. +SPDX-License-Identifier: GPL-3.0 """ import os @@ -24,25 +23,8 @@ ) from hotkdump.core.hotkdump import Hotkdump, HotkdumpParameters -from tests.utils import mock_file_ctx, fill_zeros - - -MOCK_HDR = fill_zeros( - b"KDUMP " # signature - + b"\x01\x02\x03\x04" # header_version - + fill_zeros(b"sys", 65) # system - + fill_zeros(b"node", 65) # node - + fill_zeros(b"release", 65) # release - + fill_zeros(b"#version-443", 65) # version - + fill_zeros(b"machine", 65) # machine - + fill_zeros(b"domain", 65) # domain - + b"\x02" * 6 # padding - + b"\x01\x00\x00\x00\x00\x00\x00\x00" # timestamp_sec - + b"\x02\x00\x00\x00\x00\x00\x00\x00" # timestamp_usec - + b"\x03\x00\x00\x00" # status - + b"\x00\x10\x00\x00", # block_size - 4096, -) + fill_zeros(b"", 4096) +from tests.utils import MockFileCtx, pad, MOCK_HDR + MOCK_HDR_INVALID_NO_SIG = os.urandom(4096) MOCK_VMCOREINFO = b"""key=value @@ -103,7 +85,7 @@ def test_read_cstr(self): class KdumpDiskDumpHeaderTest(TestCase): """kdump header parsing tests""" - @mock.patch("builtins.open", mock_file_ctx(bytes=MOCK_HDR, name="name")) + @mock.patch("builtins.open", MockFileCtx(file_bytes=MOCK_HDR, name="name")) def test_kdump_hdr(self): """Test kdump file header parsing with a correct header. @@ -125,19 +107,19 @@ def test_from_bytes_io(self, mfile): fake_file_content = ( b"KDUMP " # signature + b"\x01\x00\x00\x00" # header_version - + fill_zeros(b"Linux", 65) # system - + fill_zeros(b"node", 65) # node - + fill_zeros(b"release", 65) # release - + fill_zeros(b"version", 65) # version - + fill_zeros(b"machine", 65) # machine - + fill_zeros(b"domain", 65) # domain + + pad(b"Linux", 65) # system + + pad(b"node", 65) # node + + pad(b"release", 65) # release + + pad(b"version", 65) # version + + pad(b"machine", 65) # machine + + pad(b"domain", 65) # domain + b"\0" * 6 # padding + b"\x01\x00\x00\x00\x00\x00\x00\x00" # timestamp_sec + b"\x02\x00\x00\x00\x00\x00\x00\x00" # timestamp_usec + b"\x03\x00\x00\x00" # status + b"\x04\x00\x00\x00" # block_size ) - fake_file = mock_file_ctx(fake_file_content, "test") + fake_file = MockFileCtx(fake_file_content, "test") mfile.return_value = fake_file with fake_file as fd: @@ -213,7 +195,7 @@ def test_vmcoreinfo_parse(self): self.assertEqual(v.get("TEST(ABCD)"), "EFGHI") self.assertEqual(v.get("KEY"), "VALUE") - @mock.patch("builtins.open", mock_file_ctx(bytes=MOCK_VMCOREINFO, name="name")) + @mock.patch("builtins.open", MockFileCtx(file_bytes=MOCK_VMCOREINFO, name="name")) def test_vmcoreinfo_from_file(self): """Check if VMCoreInfo class can read from a file.""" with open("a", "rb") as f: @@ -234,19 +216,19 @@ def test_init_valid_kdump_file_flattened(self, mfile): $$=@@ """ fake_file_content = ( - fill_zeros(b"makedumpfile\0\0\0\0", 4096) + pad(b"makedumpfile\0\0\0\0", 4096) # makedumpfile_data_header + struct.pack(">q", 0) # offset + struct.pack(">q", 464) # size - + fill_zeros( + + pad( b"KDUMP " # signature + struct.pack("q", 4096) # offsert + struct.pack(">q", 4096) # size - + fill_zeros( + + pad( struct.pack(" None: - self.bytes = bytes + def __init__(self, file_bytes, name) -> None: + self.bytes = file_bytes self.name = name self.io = None @@ -72,22 +96,26 @@ def __call__(self, *args, **kwargs): return self def write(self, *args, **kwargs): - pass + """no-op""" -class mock_stat_obj: +class MockStatObj: + """Poor man's stat object.""" def __init__(self, name, mock_data) -> None: self.name = name self.mock_data = mock_data @property def st_atime(self): + """mock st_atime""" return self.mock_data[self.name]["atime"] @property def st_size(self): + """mock st_size""" return self.mock_data[self.name]["size"] @property def st_mode(self): + """mock st_mode""" return 16877 diff --git a/tox.ini b/tox.ini index f0b2a53..376bedc 100644 --- a/tox.ini +++ b/tox.ini @@ -1,8 +1,11 @@ [tox] -env_list = py{36,37,38,39,310,311,312},pylint +env_list = py{36,37,38,39,310,311,312},pylint,ruff skipsdist = true [testenv] +unit_tests = {toxinidir}/tests/ +module_files = + {toxinidir}/hotkdump allowlist_externals = py36: bash setenv = @@ -19,11 +22,20 @@ deps = # Note that this is awkwardly installs the package # itself and not only [optional-dependencies.testing]. # see: https://github.com/pypa/pip/issues/11440 - py37,py38,py39,py310,py311,py312,pylint: .[testing] # Install & test dependencies + py{36,37,38,39,310,311,312},pylint,ruff: .[testing] # Install & test dependencies commands = py36: pip3 install toml py36: bash -c "pip3 install $(python extras/py36-all-requirements.py)" pytest {posargs} [testenv:pylint] -commands = pylint --recursive=y -v {toxinidir}/hotkdump +commands = + pylint --recursive=y -v {posargs:{[testenv]module_files}} + # It's fine for unit tests to not have docstrings. Also, pytest does not like static test cases. + pylint --recursive=y -v {posargs:{[testenv]unit_tests}} --disable=missing-function-docstring,missing-class-docstring,no-self-use + +[testenv:ruff] +commands = + # ruff is not compatible with py36 atm. + py{37,38,39,310,311,312}: ruff check {posargs:{[testenv]module_files}} + py{37,38,39,310,311,312}: ruff check {posargs:{[testenv]unit_tests}} \ No newline at end of file