Commit aa1c0555 authored by Daniel Latypov's avatar Daniel Latypov Committed by Shuah Khan
Browse files

kunit: tool: simplify code since build_dir can't be None 



--build_dir is set to a default of '.kunit' since commit ddbd60c7
("kunit: use --build_dir=.kunit as default"), but even before then it
was explicitly set to ''.

So outside of one unit test, there was no way for the build_dir to be
ever be None, and we can simplify code by fixing the unit test and
enforcing that via updated type annotations.

E.g. this lets us drop `get_file_path()` since it's now exactly
equivalent to os.path.join().

Note: there's some `if build_dir` checks that also fail if build_dir is
explicitly set to '' that just guard against passing "O=" to make.
But running `make O=` works just fine, so drop these checks.

Signed-off-by: default avatarDaniel Latypov <dlatypov@google.com>
Reviewed-by: default avatarDavid Gow <davidgow@google.com>
Reviewed-by: default avatarBrendan Higgins <brendanhiggins@google.com>
Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
parent e6f61920

tools/testing/kunit/kunit_json.py

+3 −5
Original line number Diff line number Diff line
@@ -12,12 +12,11 @@ import os 
import kunit_parser



from kunit_parser import Test, TestStatus

from typing import Any, Dict, Optional

from typing import Any, Dict



JsonObj = Dict[str, Any]



def _get_group_json(test: Test, def_config: str,

		build_dir: Optional[str]) -> JsonObj:

def _get_group_json(test: Test, def_config: str, build_dir: str) -> JsonObj:

	sub_groups = []  # List[JsonObj]

	test_cases = []  # List[JsonObj]
@@ -50,8 +49,7 @@ def _get_group_json(test: Test, def_config: str, 
	}

	return test_group



def get_json_result(test: Test, def_config: str,

		build_dir: Optional[str]) -> str:

def get_json_result(test: Test, def_config: str, build_dir: str) -> str:

	test_group = _get_group_json(test, def_config, build_dir)

	test_group["name"] = "KUnit Test Group"

	return json.dumps(test_group, indent=4)

tools/testing/kunit/kunit_kernel.py

+20 −31
Original line number Diff line number Diff line
@@ -29,11 +29,6 @@ OUTFILE_PATH = 'test.log' 
ABS_TOOL_PATH = os.path.abspath(os.path.dirname(__file__))

QEMU_CONFIGS_DIR = os.path.join(ABS_TOOL_PATH, 'qemu_configs')



def get_file_path(build_dir, default):

	if build_dir:

		default = os.path.join(build_dir, default)

	return default



class ConfigError(Exception):

	"""Represents an error trying to configure the Linux kernel."""
@@ -60,17 +55,15 @@ class LinuxSourceTreeOperations(object): 
	def make_arch_qemuconfig(self, kconfig: kunit_config.Kconfig) -> None:

		pass



	def make_allyesconfig(self, build_dir, make_options) -> None:

	def make_allyesconfig(self, build_dir: str, make_options) -> None:

		raise ConfigError('Only the "um" arch is supported for alltests')



	def make_olddefconfig(self, build_dir, make_options) -> None:

		command = ['make', 'ARCH=' + self._linux_arch, 'olddefconfig']

	def make_olddefconfig(self, build_dir: str, make_options) -> None:

		command = ['make', 'ARCH=' + self._linux_arch, 'O=' + build_dir, 'olddefconfig']

		if self._cross_compile:

			command += ['CROSS_COMPILE=' + self._cross_compile]

		if make_options:

			command.extend(make_options)

		if build_dir:

			command += ['O=' + build_dir]

		print('Populating config with:\n$', ' '.join(command))

		try:

			subprocess.check_output(command, stderr=subprocess.STDOUT)
@@ -79,14 +72,12 @@ class LinuxSourceTreeOperations(object): 
		except subprocess.CalledProcessError as e:

			raise ConfigError(e.output.decode())



	def make(self, jobs, build_dir, make_options) -> None:

		command = ['make', 'ARCH=' + self._linux_arch, '--jobs=' + str(jobs)]

	def make(self, jobs, build_dir: str, make_options) -> None:

		command = ['make', 'ARCH=' + self._linux_arch, 'O=' + build_dir, '--jobs=' + str(jobs)]

		if make_options:

			command.extend(make_options)

		if self._cross_compile:

			command += ['CROSS_COMPILE=' + self._cross_compile]

		if build_dir:

			command += ['O=' + build_dir]

		print('Building with:\n$', ' '.join(command))

		try:

			proc = subprocess.Popen(command,
@@ -144,14 +135,12 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): 
	def __init__(self, cross_compile=None):

		super().__init__(linux_arch='um', cross_compile=cross_compile)



	def make_allyesconfig(self, build_dir, make_options) -> None:

	def make_allyesconfig(self, build_dir: str, make_options) -> None:

		kunit_parser.print_with_timestamp(

			'Enabling all CONFIGs for UML...')

		command = ['make', 'ARCH=um', 'allyesconfig']

		command = ['make', 'ARCH=um', 'O=' + build_dir, 'allyesconfig']

		if make_options:

			command.extend(make_options)

		if build_dir:

			command += ['O=' + build_dir]

		process = subprocess.Popen(

			command,

			stdout=subprocess.DEVNULL,
@@ -168,24 +157,24 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations): 


	def start(self, params: List[str], build_dir: str) -> subprocess.Popen:

		"""Runs the Linux UML binary. Must be named 'linux'."""

		linux_bin = get_file_path(build_dir, 'linux')

		linux_bin = os.path.join(build_dir, 'linux')

		return subprocess.Popen([linux_bin] + params,

					   stdin=subprocess.PIPE,

					   stdout=subprocess.PIPE,

					   stderr=subprocess.STDOUT,

					   text=True, errors='backslashreplace')



def get_kconfig_path(build_dir) -> str:

	return get_file_path(build_dir, KCONFIG_PATH)

def get_kconfig_path(build_dir: str) -> str:

	return os.path.join(build_dir, KCONFIG_PATH)



def get_kunitconfig_path(build_dir) -> str:

	return get_file_path(build_dir, KUNITCONFIG_PATH)

def get_kunitconfig_path(build_dir: str) -> str:

	return os.path.join(build_dir, KUNITCONFIG_PATH)



def get_old_kunitconfig_path(build_dir) -> str:

	return get_file_path(build_dir, OLD_KUNITCONFIG_PATH)

def get_old_kunitconfig_path(build_dir: str) -> str:

	return os.path.join(build_dir, OLD_KUNITCONFIG_PATH)



def get_outfile_path(build_dir) -> str:

	return get_file_path(build_dir, OUTFILE_PATH)

def get_outfile_path(build_dir: str) -> str:

	return os.path.join(build_dir, OUTFILE_PATH)



def get_source_tree_ops(arch: str, cross_compile: Optional[str]) -> LinuxSourceTreeOperations:

	config_path = os.path.join(QEMU_CONFIGS_DIR, arch + '.py')
@@ -269,7 +258,7 @@ class LinuxSourceTree(object): 
			return False

		return True



	def validate_config(self, build_dir) -> bool:

	def validate_config(self, build_dir: str) -> bool:

		kconfig_path = get_kconfig_path(build_dir)

		validated_kconfig = kunit_config.parse_file(kconfig_path)

		if self._kconfig.is_subset_of(validated_kconfig):
@@ -284,7 +273,7 @@ class LinuxSourceTree(object): 
		logging.error(message)

		return False



	def build_config(self, build_dir, make_options) -> bool:

	def build_config(self, build_dir: str, make_options) -> bool:

		kconfig_path = get_kconfig_path(build_dir)

		if build_dir and not os.path.exists(build_dir):

			os.mkdir(build_dir)
@@ -312,7 +301,7 @@ class LinuxSourceTree(object): 
		old_kconfig = kunit_config.parse_file(old_path)

		return old_kconfig.entries() != self._kconfig.entries()



	def build_reconfig(self, build_dir, make_options) -> bool:

	def build_reconfig(self, build_dir: str, make_options) -> bool:

		"""Creates a new .config if it is not a subset of the .kunitconfig."""

		kconfig_path = get_kconfig_path(build_dir)

		if not os.path.exists(kconfig_path):
@@ -327,7 +316,7 @@ class LinuxSourceTree(object): 
		os.remove(kconfig_path)

		return self.build_config(build_dir, make_options)



	def build_kernel(self, alltests, jobs, build_dir, make_options) -> bool:

	def build_kernel(self, alltests, jobs, build_dir: str, make_options) -> bool:

		try:

			if alltests:

				self._ops.make_allyesconfig(build_dir, make_options)

tools/testing/kunit/kunit_tool_test.py

+1 −1
Original line number Diff line number Diff line
@@ -469,7 +469,7 @@ class KUnitJsonTest(unittest.TestCase): 
			json_obj = kunit_json.get_json_result(

				test=test_result,

				def_config='kunit_defconfig',

				build_dir=None)

				build_dir='.kunit')

		return json.loads(json_obj)



	def test_failed_test_json(self):