lkml.org 
[lkml]   [2020]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] kunit: Run all KUnit tests through allyesconfig
On Tue, Feb 25, 2020 at 12:11:30PM -0800, Heidi Fahim wrote:
> Implemented the functionality to run all KUnit tests through kunit_tool
> by specifying an --alltests flag, which builds UML with allyesconfig
> enabled, and consequently runs every KUnit test. Two new methods have
> been added to kunit_kernel: make_allyesconfig and run_allconfig.
> Firstly, if --alltests is specified, kunit.py triggers build_um_kernel
> which sets jobs to the max number of cpus on the user's computer. This
> is done to shorten the long running time it takes to build and start a
> kernel with all configs enabled. It then calls the make command,
> disables the broken configs that would otherwise prevent UML from
> building, then starts the kernel with all possible configurations
> enabled. All stdout and stderr is sent to test.log and read from there
> then fed through kunit_parser to parse the tests to the user. Also added
> a signal_handler to clean the config in case kunit is interrupted while
> running.
> Tested: Run under different conditions such as testing with
> --raw_output, testing program interrupt then immediately running kunit
> again without --alltests and making sure to clean the configs. Formal
> unit tests will be submitted in a future patchset.

I saw that you added some unit tests below. Do those not provide
coverage for your new flag?

> Signed-off-by: Heidi Fahim <heidifahim@google.com>

Since you have to send another revision of this patchset anyway, would
you mind rebasing this patchset on linux-kselftest/kunit?

Other than that and a couple of minor nits. This all looks good to me.

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>

> ---
> .../kunit/configs/broken_on_uml.config | 38 ++++++++++
> tools/testing/kunit/kunit.py | 37 ++++++----
> tools/testing/kunit/kunit_kernel.py | 71 +++++++++++++------
> tools/testing/kunit/kunit_parser.py | 1 +
> tools/testing/kunit/kunit_tool_test.py | 17 +++--
> 5 files changed, 122 insertions(+), 42 deletions(-)
> create mode 100644 tools/testing/kunit/configs/broken_on_uml.config
>
> diff --git a/tools/testing/kunit/configs/broken_on_uml.config b/tools/testing/kunit/configs/broken_on_uml.config
> new file mode 100644
> index 000000000000..6d746588d46e
> --- /dev/null
> +++ b/tools/testing/kunit/configs/broken_on_uml.config
> @@ -0,0 +1,38 @@

nit: Could you add a comment here explaining that these configs are
currently broken on UML?

> +# CONFIG_STATIC_LINK is not set
> +# CONFIG_UML_NET_VECTOR is not set
> +# CONFIG_UML_NET_VDE is not set
> +# CONFIG_UML_NET_PCAP is not set
> +# CONFIG_NET_PTP_CLASSIFY is not set
> +# CONFIG_IP_VS is not set
> +# CONFIG_BRIDGE_EBT_BROUTE is not set
> +# CONFIG_BRIDGE_EBT_T_FILTER is not set
> +# CONFIG_BRIDGE_EBT_T_NAT is not set
> +# CONFIG_MTD_NAND_CADENCE is not set
> +# CONFIG_MTD_NAND_NANDSIM is not set
> +# CONFIG_BLK_DEV_NULL_BLK is not set
> +# CONFIG_BLK_DEV_RAM is not set
> +# CONFIG_SCSI_DEBUG is not set
> +# CONFIG_NET_VENDOR_XILINX is not set
> +# CONFIG_NULL_TTY is not set
> +# CONFIG_PTP_1588_CLOCK is not set
> +# CONFIG_PINCTRL_EQUILIBRIUM is not set
> +# CONFIG_DMABUF_SELFTESTS is not set
> +# CONFIG_COMEDI is not set
> +# CONFIG_XIL_AXIS_FIFO is not set
> +# CONFIG_EXFAT_FS is not set
> +# CONFIG_STM_DUMMY is not set
> +# CONFIG_FSI_MASTER_ASPEED is not set
> +# CONFIG_JFS_FS is not set
> +# CONFIG_UBIFS_FS is not set
> +# CONFIG_CRAMFS is not set
> +# CONFIG_CRYPTO_DEV_SAFEXCEL is not set
> +# CONFIG_CRYPTO_DEV_AMLOGIC_GXL is not set
> +# CONFIG_KCOV is not set
> +# CONFIG_LKDTM is not set
> +# CONFIG_REED_SOLOMON_TEST is not set
> +# CONFIG_TEST_RHASHTABLE is not set
> +# CONFIG_TEST_MEMINIT is not set
> +# CONFIG_NETWORK_PHY_TIMESTAMPING is not set
> +# CONFIG_DEBUG_INFO_BTF is not set
> +# CONFIG_PTP_1588_CLOCK_INES is not set
> +# CONFIG_QCOM_CPR is not set
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index e59eb9e7f923..37bd20a2a1c5 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -22,7 +22,9 @@ import kunit_parser
>
> KunitResult = namedtuple('KunitResult', ['status','result'])
>
> -KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', 'build_dir', 'defconfig'])
> +KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs',
> + 'build_dir', 'defconfig',
> + 'alltests'])
>
> class KunitStatus(Enum):
> SUCCESS = auto()
> @@ -33,7 +35,7 @@ class KunitStatus(Enum):
> def create_default_kunitconfig():
> if not os.path.exists(kunit_kernel.kunitconfig_path):
> shutil.copyfile('arch/um/configs/kunit_defconfig',
> - kunit_kernel.kunitconfig_path)
> + kunit_kernel.KUNITCONFIG_PATH)
>
> def run_tests(linux: kunit_kernel.LinuxSourceTree,
> request: KunitRequest) -> KunitResult:
> @@ -46,24 +48,24 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree,
> kunit_parser.print_with_timestamp('Building KUnit Kernel ...')
>
> build_start = time.time()
> - success = linux.build_um_kernel(request.jobs, request.build_dir)
> + success = linux.build_um_kernel(request.alltests,
> + request.jobs,
> + request.build_dir)
> build_end = time.time()
> if not success:
> return KunitResult(KunitStatus.BUILD_FAILURE, 'could not build kernel')
>
> kunit_parser.print_with_timestamp('Starting KUnit Kernel ...')
> test_start = time.time()
> -
> - test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS,
> - [],
> - 'Tests not Parsed.')
> + kunit_output = linux.run_kernel(
> + timeout=None if request.alltests else request.timeout,
> + alltests=request.alltests,
> + build_dir=request.build_dir)
> if request.raw_output:
> - kunit_parser.raw_output(
> - linux.run_kernel(timeout=request.timeout,
> - build_dir=request.build_dir))
> + raw_output = kunit_parser.raw_output(kunit_output)
> + isolated = list(kunit_parser.isolate_kunit_output(raw_output))
> + test_result = kunit_parser.parse_test_result(isolated)
> else:
> - kunit_output = linux.run_kernel(timeout=request.timeout,
> - build_dir=request.build_dir)
> test_result = kunit_parser.parse_run_tests(kunit_output)
> test_end = time.time()
>
> @@ -111,15 +113,19 @@ def main(argv, linux=None):
> help='Uses a default .kunitconfig.',
> action='store_true')
>
> + run_parser.add_argument('--alltests',
> + help='Run all KUnit tests through allyesconfig',
> + action='store_true')
> +
> cli_args = parser.parse_args(argv)
>
> if cli_args.subcommand == 'run':
> if cli_args.build_dir:
> if not os.path.exists(cli_args.build_dir):
> os.mkdir(cli_args.build_dir)
> - kunit_kernel.kunitconfig_path = os.path.join(
> + kunit_kernel.KUNITCONFIG_PATH = os.path.join(
> cli_args.build_dir,
> - kunit_kernel.kunitconfig_path)
> + kunit_kernel.KUNITCONFIG_PATH)
>
> if cli_args.defconfig:
> create_default_kunitconfig()
> @@ -131,7 +137,8 @@ def main(argv, linux=None):
> cli_args.timeout,
> cli_args.jobs,
> cli_args.build_dir,
> - cli_args.defconfig)
> + cli_args.defconfig,
> + cli_args.alltests)
> result = run_tests(linux, request)
> if result.status != KunitStatus.SUCCESS:
> sys.exit(1)
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index cc5d844ecca1..2b0de7d52110 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -10,11 +10,16 @@
> import logging
> import subprocess
> import os
> +import signal
> +
> +from contextlib import ExitStack
>
> import kunit_config
> +import kunit_parser
>
> KCONFIG_PATH = '.config'
> -kunitconfig_path = '.kunitconfig'
> +KUNITCONFIG_PATH = '.kunitconfig'

Can you leave this lowercase? It looks like we modify this in another
file, I think that leaving it lowercase helps indicate that this is not
a constant.

> +BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
>
> class ConfigError(Exception):
> """Represents an error trying to configure the Linux kernel."""
> @@ -40,12 +45,29 @@ class LinuxSourceTreeOperations(object):
> if build_dir:
> command += ['O=' + build_dir]
> try:
> - subprocess.check_output(command)
> + subprocess.check_output(command, stderr=subprocess.PIPE)
> except OSError as e:
> raise ConfigError('Could not call make command: ' + e)
> except subprocess.CalledProcessError as e:
> raise ConfigError(e.output)
>
> + def make_allyesconfig(self):
> + kunit_parser.print_with_timestamp(
> + 'Enabling all CONFIGs for UML...')
> + process = subprocess.Popen(
> + ['make', 'ARCH=um', 'allyesconfig'],
> + stdout=subprocess.DEVNULL,
> + stderr=subprocess.STDOUT)
> + process.wait()
> + kunit_parser.print_with_timestamp(
> + 'Disabling broken configs to run KUnit tests...')
> + with ExitStack() as es:
> + config = open(KCONFIG_PATH, 'a')
> + disable = open(BROKEN_ALLCONFIG_PATH, 'r').read()
> + config.write(disable)
> + kunit_parser.print_with_timestamp(
> + 'Starting Kernel with all configs takes a few minutes...')
> +
> def make(self, jobs, build_dir):
> command = ['make', 'ARCH=um', '--jobs=' + str(jobs)]
> if build_dir:
> @@ -57,19 +79,16 @@ class LinuxSourceTreeOperations(object):
> except subprocess.CalledProcessError as e:
> raise BuildError(e.output)
>
> - def linux_bin(self, params, timeout, build_dir):
> + def linux_bin(self, params, timeout, build_dir, outfile):
> """Runs the Linux UML binary. Must be named 'linux'."""
> linux_bin = './linux'
> if build_dir:
> linux_bin = os.path.join(build_dir, 'linux')
> - process = subprocess.Popen(
> - [linux_bin] + params,
> - stdin=subprocess.PIPE,
> - stdout=subprocess.PIPE,
> - stderr=subprocess.PIPE)
> - process.wait(timeout=timeout)
> - return process
> -
> + with open(outfile, 'w') as output:
> + process = subprocess.Popen([linux_bin] + params,
> + stdout=output,
> + stderr=subprocess.STDOUT)
> + process.wait(timeout)
>
> def get_kconfig_path(build_dir):
> kconfig_path = KCONFIG_PATH
> @@ -82,8 +101,9 @@ class LinuxSourceTree(object):
>
> def __init__(self):
> self._kconfig = kunit_config.Kconfig()
> - self._kconfig.read_from_file(kunitconfig_path)
> + self._kconfig.read_from_file(KUNITCONFIG_PATH)
> self._ops = LinuxSourceTreeOperations()
> + signal.signal(signal.SIGINT, self.signal_handler)
>
> def clean(self):
> try:
> @@ -126,7 +146,10 @@ class LinuxSourceTree(object):
> print('Generating .config ...')
> return self.build_config(build_dir)
>
> - def build_um_kernel(self, jobs, build_dir):
> + def build_um_kernel(self, alltests, jobs, build_dir):
> + if alltests:
> + jobs = os.cpu_count()
> + self._ops.make_allyesconfig()
> try:
> self._ops.make_olddefconfig(build_dir)
> self._ops.make(jobs, build_dir)
> @@ -140,10 +163,18 @@ class LinuxSourceTree(object):
> return False
> return True
>
> - def run_kernel(self, args=[], timeout=None, build_dir=''):
> - args.extend(['mem=256M'])
> - process = self._ops.linux_bin(args, timeout, build_dir)
> - with open(os.path.join(build_dir, 'test.log'), 'w') as f:
> - for line in process.stdout:
> - f.write(line.rstrip().decode('ascii') + '\n')
> - yield line.rstrip().decode('ascii')
> + def run_kernel(self, args=[], alltests=False, build_dir='', timeout=None):
> + args.extend(['mem=1G']) if alltests else args.extend(['mem=256M'])
> + outfile = 'test.log'
> + self._ops.linux_bin(args, timeout, build_dir, outfile)
> + subprocess.call(['stty', 'sane'])
> + if alltests:
> + self.clean()
> + with open(outfile, 'r') as file:
> + for line in file:
> + yield line
> +
> + def signal_handler(self, sig, frame):
> + logging.error('Build interruption occurred. Cleaning .config.')
> + subprocess.call(['stty', 'sane'])
> + self.clean()
> \ No newline at end of file
> diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py
> index 077b21d42258..5b2051848e2f 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -65,6 +65,7 @@ def isolate_kunit_output(kernel_output):
> def raw_output(kernel_output):
> for line in kernel_output:
> print(line)
> + yield line
>
> DIVIDER = '=' * 60
>
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 0efae697f396..9ce4c5cdbdaf 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -243,7 +243,8 @@ class KUnitMainTest(unittest.TestCase):
> kunit.main(['run'], self.linux_source_mock)
> assert self.linux_source_mock.build_reconfig.call_count == 1
> assert self.linux_source_mock.run_kernel.call_count == 1
> - self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='', timeout=300)
> + self.linux_source_mock.run_kernel.assert_called_once_with(
> + alltests=False, build_dir='', timeout=300)
> self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
> def test_run_passes_args_fail(self):
> @@ -258,25 +259,27 @@ class KUnitMainTest(unittest.TestCase):
>
> def test_run_raw_output(self):
> self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
> - kunit.main(['run', '--raw_output'], self.linux_source_mock)
> + with self.assertRaises(SystemExit) as e:
> + kunit.main(['run', '--raw_output'], self.linux_source_mock)
> + assert type(e.exception) == SystemExit
> + assert e.exception.code == 1
> assert self.linux_source_mock.build_reconfig.call_count == 1
> assert self.linux_source_mock.run_kernel.call_count == 1
> - for kall in self.print_mock.call_args_list:
> - assert kall != mock.call(StrContains('Testing complete.'))
> - assert kall != mock.call(StrContains(' 0 tests run'))
>
> def test_run_timeout(self):
> timeout = 3453
> kunit.main(['run', '--timeout', str(timeout)], self.linux_source_mock)
> assert self.linux_source_mock.build_reconfig.call_count == 1
> - self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='', timeout=timeout)
> + self.linux_source_mock.run_kernel.assert_called_once_with(
> + alltests=False, build_dir='', timeout=timeout)
> self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
> def test_run_builddir(self):
> build_dir = '.kunit'
> kunit.main(['run', '--build_dir', build_dir], self.linux_source_mock)
> assert self.linux_source_mock.build_reconfig.call_count == 1
> - self.linux_source_mock.run_kernel.assert_called_once_with(build_dir=build_dir, timeout=300)
> + self.linux_source_mock.run_kernel.assert_called_once_with(
> + alltests=False, build_dir=build_dir, timeout=300)
> self.print_mock.assert_any_call(StrContains('Testing complete.'))
>
> if __name__ == '__main__':
> --
> 2.25.0.265.gbab2e86ba0-goog
>

\
 
 \ /
  Last update: 2020-02-25 23:36    [W:0.080 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site