diff mbox series

[v2] kunit: make kunit_tool accept optional path to .kunitconfig fragment

Message ID 20210201205514.3943096-1-dlatypov@google.com (mailing list archive)
State Accepted
Delegated to: Shuah Khan
Headers show
Series [v2] kunit: make kunit_tool accept optional path to .kunitconfig fragment | expand

Commit Message

Daniel Latypov Feb. 1, 2021, 8:55 p.m. UTC
Currently running tests via KUnit tool means tweaking a .kunitconfig
file, which you'd keep around locally and never commit.
This changes makes it so users can pass in a path to a kunitconfig.

One of the imagined use cases is having kunitconfig fragments in-tree
to formalize interesting sets of tests for features/subsystems, e.g.
  $ ./tools/testing/kunit/kunit.py run --kunticonfig=fs/ext4/kunitconfig

For now, this hypothetical fs/ext4/kunitconfig would contain
  CONFIG_KUNIT=y
  CONFIG_EXT4_FS=y
  CONFIG_EXT4_KUNIT_TESTS=y

At the moment, it's not hard to manually whip up this file, but as more
and more tests get added, this will get tedious.

It also opens the door to documenting how to run all the tests relevant
to a specific subsystem or feature as a simple one-liner.

This can be seen as an analogue to tools/testing/selftests/*/config
But in the case of KUnit, the tests live in the same directory as the
code-under-test, so it feels more natural to allow the kunitconfig
fragments to live anywhere. (Though, people could create a separate
directory if wanted; this patch imposes no restrictions on the path).

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---

Changes since v1: change from a positional arg to a flag --kunitconfig.
Ensure that it gets added for `kunit.py config` and all other commands.

---
 tools/testing/kunit/kunit.py           |  9 +++++---
 tools/testing/kunit/kunit_kernel.py    | 12 ++++++----
 tools/testing/kunit/kunit_tool_test.py | 32 ++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 7 deletions(-)


base-commit: 88bb507a74ea7d75fa49edd421eaa710a7d80598

Comments

Brendan Higgins Feb. 1, 2021, 9:30 p.m. UTC | #1
On Mon, Feb 1, 2021 at 12:55 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> Currently running tests via KUnit tool means tweaking a .kunitconfig
> file, which you'd keep around locally and never commit.
> This changes makes it so users can pass in a path to a kunitconfig.
>
> One of the imagined use cases is having kunitconfig fragments in-tree
> to formalize interesting sets of tests for features/subsystems, e.g.
>   $ ./tools/testing/kunit/kunit.py run --kunticonfig=fs/ext4/kunitconfig
>
> For now, this hypothetical fs/ext4/kunitconfig would contain
>   CONFIG_KUNIT=y
>   CONFIG_EXT4_FS=y
>   CONFIG_EXT4_KUNIT_TESTS=y
>
> At the moment, it's not hard to manually whip up this file, but as more
> and more tests get added, this will get tedious.
>
> It also opens the door to documenting how to run all the tests relevant
> to a specific subsystem or feature as a simple one-liner.
>
> This can be seen as an analogue to tools/testing/selftests/*/config
> But in the case of KUnit, the tests live in the same directory as the
> code-under-test, so it feels more natural to allow the kunitconfig
> fragments to live anywhere. (Though, people could create a separate
> directory if wanted; this patch imposes no restrictions on the path).
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Brendan Higgins Feb. 5, 2021, 11:46 p.m. UTC | #2
On Mon, Feb 1, 2021 at 12:55 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> Currently running tests via KUnit tool means tweaking a .kunitconfig
> file, which you'd keep around locally and never commit.
> This changes makes it so users can pass in a path to a kunitconfig.
>
> One of the imagined use cases is having kunitconfig fragments in-tree
> to formalize interesting sets of tests for features/subsystems, e.g.
>   $ ./tools/testing/kunit/kunit.py run --kunticonfig=fs/ext4/kunitconfig
>
> For now, this hypothetical fs/ext4/kunitconfig would contain
>   CONFIG_KUNIT=y
>   CONFIG_EXT4_FS=y
>   CONFIG_EXT4_KUNIT_TESTS=y
>
> At the moment, it's not hard to manually whip up this file, but as more
> and more tests get added, this will get tedious.
>
> It also opens the door to documenting how to run all the tests relevant
> to a specific subsystem or feature as a simple one-liner.
>
> This can be seen as an analogue to tools/testing/selftests/*/config
> But in the case of KUnit, the tests live in the same directory as the
> code-under-test, so it feels more natural to allow the kunitconfig
> fragments to live anywhere. (Though, people could create a separate
> directory if wanted; this patch imposes no restrictions on the path).
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

Tested-by: Brendan Higgins <brendanhiggins@google.com>
diff mbox series

Patch

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index e808a47c839b..02871a363f76 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -182,6 +182,9 @@  def add_common_opts(parser) -> None:
 	parser.add_argument('--alltests',
 			    help='Run all KUnit tests through allyesconfig',
 			    action='store_true')
+	parser.add_argument('--kunitconfig',
+			     help='Path to Kconfig fragment that enables KUnit tests',
+			     metavar='kunitconfig')
 
 def add_build_opts(parser) -> None:
 	parser.add_argument('--jobs',
@@ -256,7 +259,7 @@  def main(argv, linux=None):
 			os.mkdir(cli_args.build_dir)
 
 		if not linux:
-			linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
+			linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
 
 		request = KunitRequest(cli_args.raw_output,
 				       cli_args.timeout,
@@ -274,7 +277,7 @@  def main(argv, linux=None):
 			os.mkdir(cli_args.build_dir)
 
 		if not linux:
-			linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
+			linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
 
 		request = KunitConfigRequest(cli_args.build_dir,
 					     cli_args.make_options)
@@ -286,7 +289,7 @@  def main(argv, linux=None):
 			sys.exit(1)
 	elif cli_args.subcommand == 'build':
 		if not linux:
-			linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir)
+			linux = kunit_kernel.LinuxSourceTree(cli_args.build_dir, kunitconfig_path=cli_args.kunitconfig)
 
 		request = KunitBuildRequest(cli_args.jobs,
 					    cli_args.build_dir,
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 2076a5a2d060..0b461663e7d9 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -123,7 +123,7 @@  def get_outfile_path(build_dir) -> str:
 class LinuxSourceTree(object):
 	"""Represents a Linux kernel source tree with KUnit tests."""
 
-	def __init__(self, build_dir: str, load_config=True, defconfig=DEFAULT_KUNITCONFIG_PATH) -> None:
+	def __init__(self, build_dir: str, load_config=True, kunitconfig_path='') -> None:
 		signal.signal(signal.SIGINT, self.signal_handler)
 
 		self._ops = LinuxSourceTreeOperations()
@@ -131,9 +131,13 @@  class LinuxSourceTree(object):
 		if not load_config:
 			return
 
-		kunitconfig_path = get_kunitconfig_path(build_dir)
-		if not os.path.exists(kunitconfig_path):
-			shutil.copyfile(defconfig, kunitconfig_path)
+		if kunitconfig_path:
+			if not os.path.exists(kunitconfig_path):
+				raise ConfigError(f'Specified kunitconfig ({kunitconfig_path}) does not exist')
+		else:
+			kunitconfig_path = get_kunitconfig_path(build_dir)
+			if not os.path.exists(kunitconfig_path):
+				shutil.copyfile(DEFAULT_KUNITCONFIG_PATH, kunitconfig_path)
 
 		self._kconfig = kunit_config.Kconfig()
 		self._kconfig.read_from_file(kunitconfig_path)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index b593f4448e83..22f50b931138 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -12,6 +12,7 @@  from unittest import mock
 import tempfile, shutil # Handling test_tmpdir
 
 import json
+import signal
 import os
 
 import kunit_config
@@ -250,6 +251,23 @@  class KUnitParserTest(unittest.TestCase):
 				result.status)
 			self.assertEqual('kunit-resource-test', result.suites[0].name)
 
+class LinuxSourceTreeTest(unittest.TestCase):
+
+	def setUp(self):
+		mock.patch.object(signal, 'signal').start()
+		self.addCleanup(mock.patch.stopall)
+
+	def test_invalid_kunitconfig(self):
+		with self.assertRaisesRegex(kunit_kernel.ConfigError, 'nonexistent.* does not exist'):
+			kunit_kernel.LinuxSourceTree('', kunitconfig_path='/nonexistent_file')
+
+	def test_valid_kunitconfig(self):
+		with tempfile.NamedTemporaryFile('wt') as kunitconfig:
+			tree = kunit_kernel.LinuxSourceTree('', kunitconfig_path=kunitconfig.name)
+
+	# TODO: add more test cases.
+
+
 class KUnitJsonTest(unittest.TestCase):
 
 	def _json_for(self, log_file):
@@ -399,5 +417,19 @@  class KUnitMainTest(unittest.TestCase):
 		self.linux_source_mock.run_kernel.assert_called_once_with(build_dir=build_dir, timeout=300)
 		self.print_mock.assert_any_call(StrContains('Testing complete.'))
 
+	@mock.patch.object(kunit_kernel, 'LinuxSourceTree')
+	def test_run_kunitconfig(self, mock_linux_init):
+		mock_linux_init.return_value = self.linux_source_mock
+		kunit.main(['run', '--kunitconfig=mykunitconfig'])
+		# Just verify that we parsed and initialized it correctly here.
+		mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
+
+	@mock.patch.object(kunit_kernel, 'LinuxSourceTree')
+	def test_config_kunitconfig(self, mock_linux_init):
+		mock_linux_init.return_value = self.linux_source_mock
+		kunit.main(['config', '--kunitconfig=mykunitconfig'])
+		# Just verify that we parsed and initialized it correctly here.
+		mock_linux_init.assert_called_once_with('.kunit', kunitconfig_path='mykunitconfig')
+
 if __name__ == '__main__':
 	unittest.main()