diff mbox series

kunit: tool: make `build` subcommand also reconfigure if needed

Message ID 20211202203927.2004527-1-dlatypov@google.com
State Superseded
Headers show
Series kunit: tool: make `build` subcommand also reconfigure if needed | expand

Commit Message

Daniel Latypov Dec. 2, 2021, 8:39 p.m. UTC
If I created a kunitconfig file that was incomplete, then
$ ./tools/testing/kunit/kunit.py build --kunitconfig=my_kunitconfig
would silently drop all the options with unmet dependencies!

This is because it doesn't do the config check that `kunit.py config`
does.

So if I want to safely build a kernel for testing, I have to do
$ ./tools/testing/kunit/kunit.py config <flags>
$ ./tools/testing/kunit/kunit.py build <flags, again>

It seems unlikely that any user of kunit.py would want the current
`build` semantics.
So make it effectively do `kunit.py config` + `kunit.py build`.

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
Note: this patch depends on https://lore.kernel.org/linux-kselftest/20211009015406.1311319-1-dlatypov@google.com/
That patch simplifies all these functions by reducing the amount of
boilerplate needed to convert between XXXRequest types.

But we can rewrite this so it doesn't have that dep at the cost of
the patch becoming more verbose.
---
 tools/testing/kunit/kunit.py           | 10 +++++++++-
 tools/testing/kunit/kunit_tool_test.py |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Brendan Higgins Dec. 7, 2021, 11 p.m. UTC | #1
On Thu, Dec 2, 2021 at 3:39 PM Daniel Latypov <dlatypov@google.com> wrote:
>
> If I created a kunitconfig file that was incomplete, then
> $ ./tools/testing/kunit/kunit.py build --kunitconfig=my_kunitconfig
> would silently drop all the options with unmet dependencies!
>
> This is because it doesn't do the config check that `kunit.py config`
> does.
>
> So if I want to safely build a kernel for testing, I have to do
> $ ./tools/testing/kunit/kunit.py config <flags>
> $ ./tools/testing/kunit/kunit.py build <flags, again>
>
> It seems unlikely that any user of kunit.py would want the current
> `build` semantics.
> So make it effectively do `kunit.py config` + `kunit.py build`.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>

Reviewed-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 367e29d6942b..99068532485d 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -110,6 +110,14 @@  def build_tests(linux: kunit_kernel.LinuxSourceTree,
 			   'built kernel successfully',
 			   build_end - build_start)
 
+def config_and_build_tests(linux: kunit_kernel.LinuxSourceTree,
+			   request: KunitBuildRequest) -> KunitResult:
+	config_result = config_tests(linux, request)
+	if config_result.status != KunitStatus.SUCCESS:
+		return config_result
+
+	return build_tests(linux, request)
+
 def _list_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest) -> List[str]:
 	args = ['kunit.action=list']
 	if request.kernel_args:
@@ -451,7 +459,7 @@  def main(argv, linux=None):
 					    make_options=cli_args.make_options,
 					    jobs=cli_args.jobs,
 					    alltests=cli_args.alltests)
-		result = build_tests(linux, request)
+		result = config_and_build_tests(linux, request)
 		kunit_parser.print_with_timestamp((
 			'Elapsed time: %.3fs\n') % (
 				result.elapsed_time))
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 178321d1f190..43ced874d5ad 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -418,7 +418,7 @@  class KUnitMainTest(unittest.TestCase):
 
 	def test_build_passes_args_pass(self):
 		kunit.main(['build'], self.linux_source_mock)
-		self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
+		self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
 		self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, '.kunit', None)
 		self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)