Skip to content

Commit

Permalink
project: add ruff as additional checker
Browse files Browse the repository at this point in the history
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 <mustafa.gilor@canonical.com>
  • Loading branch information
xmkg committed Aug 20, 2024
1 parent 097662a commit 707b65f
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 102 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 0 additions & 1 deletion extras/py36-all-requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions hotkdump/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 13 additions & 28 deletions tests/test_hotkdump.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand All @@ -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"""

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down
86 changes: 34 additions & 52 deletions tests/test_kdump_file_header.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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("<i", 242526) # 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"\x02" * 6 # padding
+ struct.pack("<q", 1234) # timestamp_sec
+ struct.pack("<q", 5678) # timestamp_usec
Expand All @@ -257,7 +239,7 @@ def test_init_valid_kdump_file_flattened(self, mfile):
# makedumpfile_data_header
+ struct.pack(">q", 4096) # offsert
+ struct.pack(">q", 4096) # size
+ fill_zeros(
+ pad(
struct.pack("<q", 1111) # phys_base
+ struct.pack("<i", 2222) # dump_level
+ struct.pack("<i", 3333) # split
Expand All @@ -279,7 +261,7 @@ def test_init_valid_kdump_file_flattened(self, mfile):
+ fake_vmcoreinfo
)

fake_file = mock_file_ctx(fake_file_content, "test")
fake_file = MockFileCtx(fake_file_content, "test")
mfile.return_value = fake_file

kdump_file = KdumpFile("dummy_path")
Expand Down Expand Up @@ -328,15 +310,15 @@ def test_init_valid_kdump_file_compressed(self, mfile):
"""
fake_file_content = (
# 1 page diskdump_header
fill_zeros(
pad(
b"KDUMP " # signature
+ struct.pack("<i", 242526) # 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"\x02" * 6 # padding
+ struct.pack("<q", 1234) # timestamp_sec
+ struct.pack("<q", 5678) # timestamp_usec
Expand All @@ -362,7 +344,7 @@ def test_init_valid_kdump_file_compressed(self, mfile):
+ fake_vmcoreinfo
)

fake_file = mock_file_ctx(fake_file_content, "test")
fake_file = MockFileCtx(fake_file_content, "test")
mfile.return_value = fake_file

kdump_file = KdumpFile("dummy_path")
Expand Down Expand Up @@ -406,12 +388,12 @@ def test_init_valid_kdump_file_compressed(self, mfile):
@mock.patch("builtins.open", new_callable=mock.mock_open, read_data=b"")
def test_init_invalid_signature(self, mfile):
fake_file_content = b"fakefile\0\0\0"
fake_file = mock_file_ctx(fake_file_content, "test")
fake_file = MockFileCtx(fake_file_content, "test")
mfile.return_value = fake_file
with self.assertRaises(NotAKernelCrashDumpException):
_ = KdumpFile("dummy_path")

@mock.patch("builtins.open", mock_file_ctx(bytes=MOCK_HDR_INVALID_NO_SIG, name="name"))
@mock.patch("builtins.open", MockFileCtx(file_bytes=MOCK_HDR_INVALID_NO_SIG, name="name"))
def test_kdump_hdr_no_sig(self):
"""Test kdump file header parsing with
garbage input.
Expand Down
Loading

0 comments on commit 707b65f

Please sign in to comment.