From 6ed260c250a912e779b819d4e027ae571066c777 Mon Sep 17 00:00:00 2001 From: Kairui Song Date: Thu, 21 Mar 2019 17:34:28 +0800 Subject: [PATCH 1/2] Add some unit tests This should cover most parts except UI part and Anaconda API. To run the test, ensure python3-nose is installed then run "make test" Signed-off-by: Kairui Song --- Makefile | 4 +- test/unittests/test_common.py | 57 +++++++++++++++++++++++++++ test/unittests/test_kickstart.py | 67 ++++++++++++++++++++++++++++++++ test/unittests/utils.py | 12 ++++++ 4 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 test/unittests/test_common.py create mode 100644 test/unittests/test_kickstart.py create mode 100644 test/unittests/utils.py diff --git a/Makefile b/Makefile index 1982f5b..c9b581d 100644 --- a/Makefile +++ b/Makefile @@ -77,14 +77,14 @@ test: @find . -name '*.py' -print|xargs -n1 --max-procs=$(NUM_PROCS) pylint -E 2> /dev/null @echo "[ OK ]" @echo "***Running unittests checks***" - @PYTHONPATH=. nosetests --processes=-1 -vw tests/ + @PYTHONPATH=. python3 -m nose --processes=-1 -vw test/unittests runpylint: @find . -name '*.py' -print|xargs -n1 --max-procs=$(NUM_PROCS) pylint -E 2> /dev/null @echo "[ OK ]" unittest: - PYTHONPATH=. nosetests --processes=-1 -vw tests/ + PYTHONPATH=. python3 -m nose --processes=-1 -vw test/unittests version.sh: @echo "KDUMP_ADDON_VERSION=$(VERSION)" > version.sh diff --git a/test/unittests/test_common.py b/test/unittests/test_common.py new file mode 100644 index 0000000..d06263f --- /dev/null +++ b/test/unittests/test_common.py @@ -0,0 +1,57 @@ +from utils import KdumpTestCase +from unittest.mock import patch, mock_open, MagicMock +from com_redhat_kdump import common + +SYS_CRASH_SIZE = '/sys/kernel/kexec_crash_size' +PROC_MEMINFO = '/proc/meminfo' + +X86_INFO_FIXTURE = { + SYS_CRASH_SIZE: "167772160", # 160MB + PROC_MEMINFO:"""MemTotal: 4030464 kB +""" # 4GB - 160MB +} + +AARCH64_INFO_FIXTURE = { + SYS_CRASH_SIZE: "536870912", # 512MB + PROC_MEMINFO:"""MemTotal: 66584576 kB +""" # 64GB - 512MB +} + +PPC64_INFO_FIXTURE = { + SYS_CRASH_SIZE: "1073741824", # 1024MB + PROC_MEMINFO:"""MemTotal: 66060288 kB +""" # 64GB - 1GB +} + + +class MockFileRead(MagicMock): + def __init__(self, file_read_map): + MagicMock.__init__(self, name=open, spec=open) + self.file_read_map = file_read_map + + handle = MagicMock() + handle.__enter__.return_value = handle + handle.read.return_value = None + + def reset_choose_file(filename, *args, **kwargs): + handle.read.return_value = self.file_read_map[filename] + return handle + + self.side_effect = reset_choose_file + + +class KdumpCommonTestCase(KdumpTestCase): + @patch("builtins.open", MockFileRead(X86_INFO_FIXTURE)) + @patch("blivet.arch.get_arch", return_value="x86_64") + def memory_bound_test_x86(self, _mock_read): + self.assertEqual((128, 4 * 1024 - 256, 1), common.getMemoryBounds()) + + @patch("builtins.open", MockFileRead(AARCH64_INFO_FIXTURE)) + @patch("blivet.arch.get_arch", return_value="aarch64") + def memory_bound_test_aarch64(self, _mock_read): + self.assertEqual((128, 64 * 1024 - 256, 1), common.getMemoryBounds()) + + @patch("builtins.open", MockFileRead(PPC64_INFO_FIXTURE)) + @patch("blivet.arch.get_arch", return_value="ppc64") + def memory_bound_test_ppc64(self, _mock_read): + self.assertEqual((256, 64 * 1024 - 1024, 1), common.getMemoryBounds()) diff --git a/test/unittests/test_kickstart.py b/test/unittests/test_kickstart.py new file mode 100644 index 0000000..7b61e96 --- /dev/null +++ b/test/unittests/test_kickstart.py @@ -0,0 +1,67 @@ +from utils import KdumpTestCase +from unittest.mock import patch +from com_redhat_kdump.ks.kdump import KdumpData +from pykickstart.errors import KickstartParseError +import re + + +ALL_ARGS_RE = re.compile( + r"[\s\n]*%addon\s+KdumpData\s+" + r"(--enable|--disable|--enablefadump|--reverse-mb[=|\ ]['\"]?\d+['\"][Mm]?)*" + r"\s+.*[\s\n]+%end[\s\n]*") + +def new_ks_addon_data(): + return KdumpData("KdumpData") + + +def kdump_check_ks(test, addon_data, required_arguments): + ks_str = str(addon_data) + for arg in required_arguments: + # Ensure rerquired argument is generated. + arg_regex = re.compile(r"[\s\n]*%addon\s+KdumpData.*\s+" + arg + r"\s+.*[\s\n]+%end[\s\n]*") + test.assertRegexpMatches(ks_str, arg_regex) + # Ensure no invalid argument is generated. + test.assertRegexpMatches(ks_str, ALL_ARGS_RE) + + +class KdumpKickstartTestCase(KdumpTestCase): + def new_ks_addon_data_test(self): + ks_addon_data = new_ks_addon_data() + self.assertIsNotNone(ks_addon_data) + + def ks_default_to_str_test(self, _MockMemoryBounds): + ks_addon_data = new_ks_addon_data() + kdump_check_ks(self, ks_addon_data, ["--disable"]) + + def ks_enable_to_str_test(self): + ks_addon_data = new_ks_addon_data() + ks_addon_data.enabled = True + kdump_check_ks(self, ks_addon_data, ["--enable"]) + + def ks_disable_to_str_test(self): + ks_addon_data = new_ks_addon_data() + ks_addon_data.enabled = False + kdump_check_ks(self, ks_addon_data, ["--disable"]) + + def ks_parse_enable_to_str_test(self): + ks_addon_data = new_ks_addon_data() + ks_addon_data.handle_header(0, ["--enable"]) + kdump_check_ks(self, ks_addon_data, ["--enable"]) + + def ks_parse_reserve_to_str_test(self): + ks_addon_data = new_ks_addon_data() + ks_addon_data.handle_header(0, ["--enable", "--reserve-mb=256"]) + kdump_check_ks(self, ks_addon_data, ["--enable", "--reserve-mb[=|\ ](\"256\"|'256'|256)[Mm]?"]) + + def ks_parse_fadump_to_str_test(self): + ks_addon_data = new_ks_addon_data() + ks_addon_data.handle_header(0, ['--enablefadump']) + kdump_check_ks(self, ks_addon_data, ["--enablefadump"]) + + def ks_parse_invalid_reserve_size_test(self): + with self.assertRaises(KickstartParseError) as error: + ks_addon_data = new_ks_addon_data() + ks_addon_data.handle_header(0, ['--reserve-mb=']) + with self.assertRaises(KickstartParseError) as error: + ks_addon_data = new_ks_addon_data() + ks_addon_data.handle_header(0, ['--reserve-mb=invalid']) diff --git a/test/unittests/utils.py b/test/unittests/utils.py new file mode 100644 index 0000000..64ae1a0 --- /dev/null +++ b/test/unittests/utils.py @@ -0,0 +1,12 @@ +import unittest + +from unittest.mock import patch +from com_redhat_kdump import common + +def enable_kdump_addon_in_anaconda(): + return patch('pyanaconda.flags.cmdline.getbool', return_value=True) + +class KdumpTestCase(unittest.TestCase): + def setUp(self): + # Clean up global variable that may cache test result of previous test case + common._reservedMemory = None From 22a2b63bde6621c650204d7ce8ad09612262a958 Mon Sep 17 00:00:00 2001 From: Kairui Song Date: Thu, 21 Mar 2019 21:30:45 +0800 Subject: [PATCH 2/2] Allow quoting --reserve-mb parameter Previously, when parsing the kickstart file, it didn't expected the --reserved-mb number to be quoted, but when generating kickstart lines it will quote the number. This make it very confusing, as quoted parameter currently will raise a exception of "Invalid value". Fix it by strip the quotes before parsing the int. Signed-off-by: Kairui Song --- com_redhat_kdump/ks/kdump.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/com_redhat_kdump/ks/kdump.py b/com_redhat_kdump/ks/kdump.py index 6cc664f..53878b9 100644 --- a/com_redhat_kdump/ks/kdump.py +++ b/com_redhat_kdump/ks/kdump.py @@ -117,7 +117,8 @@ def handle_header(self, lineno, args): # Validate the reserve-mb argument # Allow a final 'M' for consistency with the crashkernel kernel - # parameter. Strip it if found. + # parameter. Strip it if found. And strip quotes. + opts.reserveMB = opts.reserveMB.strip("'\"") if opts.reserveMB and opts.reserveMB[-1] == 'M': opts.reserveMB = opts.reserveMB[:-1]