2019-11-26 19:34:21

by Heidi Fahim

[permalink] [raw]
Subject: [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid

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 <[email protected]>
---
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' % (
+ ', '.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):
@@ -103,12 +116,7 @@ class LinuxSourceTree(object):
except ConfigError as e:
logging.error(e)
return False
- validated_kconfig = kunit_config.Kconfig()
- validated_kconfig.read_from_file(kconfig_path)
- if not self._kconfig.is_subset_of(validated_kconfig):
- logging.error('Provided Kconfig is not contained in validated .config!')
- return False
- return True
+ return self.validate_config(build_dir)

def build_reconfig(self, build_dir):
"""Creates a new .config if it is not a subset of the kunitconfig."""
@@ -133,12 +141,7 @@ class LinuxSourceTree(object):
except (ConfigError, BuildError) as e:
logging.error(e)
return False
- used_kconfig = kunit_config.Kconfig()
- used_kconfig.read_from_file(get_kconfig_path(build_dir))
- if not self._kconfig.is_subset_of(used_kconfig):
- logging.error('Provided Kconfig is not contained in final config!')
- return False
- return True
+ return self.validate_config(build_dir)

def run_kernel(self, args=[], timeout=None, build_dir=None):
args.extend(['mem=256M'])
--
2.24.0.432.g9d3f5f5b63-goog


2019-11-26 20:11:19

by Brendan Higgins

[permalink] [raw]
Subject: 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
<[email protected]> 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 <[email protected]>

Reviewed-by: Brendan Higgins <[email protected]>
Tested-by: Brendan Higgins <[email protected]>

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!

2019-11-26 22:38:02

by Heidi Fahim

[permalink] [raw]
Subject: [PATCH v2] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid

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 <[email protected]>
Reviewed-by: Brendan Higgins <[email protected]>
Tested-by: Brendan Higgins <[email protected]>
---
Addressed Brendan's nit for error message

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

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index bf3876835331..e1df9aad778f 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -93,6 +93,20 @@ 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. Following fields found in kunitconfig, ' \
+ 'but not in .config: %s' % (
+ ', '.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):
@@ -103,12 +117,7 @@ class LinuxSourceTree(object):
except ConfigError as e:
logging.error(e)
return False
- validated_kconfig = kunit_config.Kconfig()
- validated_kconfig.read_from_file(kconfig_path)
- if not self._kconfig.is_subset_of(validated_kconfig):
- logging.error('Provided Kconfig is not contained in validated .config!')
- return False
- return True
+ return self.validate_config(build_dir)

def build_reconfig(self, build_dir):
"""Creates a new .config if it is not a subset of the kunitconfig."""
@@ -133,12 +142,7 @@ class LinuxSourceTree(object):
except (ConfigError, BuildError) as e:
logging.error(e)
return False
- used_kconfig = kunit_config.Kconfig()
- used_kconfig.read_from_file(get_kconfig_path(build_dir))
- if not self._kconfig.is_subset_of(used_kconfig):
- logging.error('Provided Kconfig is not contained in final config!')
- return False
- return True
+ return self.validate_config(build_dir)

def run_kernel(self, args=[], timeout=None, build_dir=None):
args.extend(['mem=256M'])
--
2.24.0.432.g9d3f5f5b63-goog