Re: [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid

+David Gow

On Tue, Nov 26, 2019 at 11:33 AM 'Heidi Fahim' via KUnit Development kunit-dev@googlegroups.com wrote:

Previous error message for invalid kunitconfig was vague. Added to it so that it lists invalid fields and prompts for them to be removed. Added validate_config function returning whether or not this kconfig is valid.

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

Reviewed-by: Brendan Higgins brendanhiggins@google.com Tested-by: Brendan Higgins brendanhiggins@google.com

Looks good to me other than one minor nit below.

---

tools/testing/kunit/kunit_kernel.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index bf3876835331..010d3f5030d2 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -93,6 +93,19 @@ class LinuxSourceTree(object): return False return True

•   def validate_config(self, build_dir):
  

•           kconfig_path = get_kconfig_path(build_dir)
  

•           validated_kconfig = kunit_config.Kconfig()
  

•           validated_kconfig.read_from_file(kconfig_path)
  

•           if not self._kconfig.is_subset_of(validated_kconfig):
  

•                   invalid = self._kconfig.entries() - validated_kconfig.entries()
  

•                   message = 'Provided Kconfig is not contained in validated .config. Invalid fields found in kunitconfig: %s' % (
  

nit: Rather than "Invalid fields found in kunitconfig", how about something like "Following fields found in kunitconfig, but not .config:"?

•                           ', '.join([str(e) for e in invalid])
  

•                   )
  

•                   logging.error(message)
  

•                   return False
  

•           return True
  

•   def build_config(self, build_dir):
            kconfig_path = get_kconfig_path(build_dir)
            if build_dir and not os.path.exists(build_dir):
  

Thanks for the patch!