Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid
@ 2019-11-26 19:33 Heidi Fahim
  2019-11-26 20:07 ` Brendan Higgins
  2019-11-26 22:36 ` [PATCH v2] " Heidi Fahim
  0 siblings, 2 replies; 3+ messages in thread
From: Heidi Fahim @ 2019-11-26 19:33 UTC (permalink / raw)
  To: brendanhiggins, shuah
  Cc: linux-kselftest, kunit-dev, linux-kernel, Heidi Fahim

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>
---
 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid
  2019-11-26 19:33 [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid Heidi Fahim
@ 2019-11-26 20:07 ` Brendan Higgins
  2019-11-26 22:36 ` [PATCH v2] " Heidi Fahim
  1 sibling, 0 replies; 3+ messages in thread
From: Brendan Higgins @ 2019-11-26 20:07 UTC (permalink / raw)
  To: Heidi Fahim, David Gow
  Cc: shuah, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development,
	Linux Kernel Mailing List

+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!

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid
  2019-11-26 19:33 [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid Heidi Fahim
  2019-11-26 20:07 ` Brendan Higgins
@ 2019-11-26 22:36 ` " Heidi Fahim
  1 sibling, 0 replies; 3+ messages in thread
From: Heidi Fahim @ 2019-11-26 22:36 UTC (permalink / raw)
  To: brendanhiggins, shuah, davidgow
  Cc: linux-kselftest, kunit-dev, linux-kernel, Heidi Fahim

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>
---
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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26 19:33 [PATCH] kunit: test: Improve error messages for kunit_tool when kunitconfig is invalid Heidi Fahim
2019-11-26 20:07 ` Brendan Higgins
2019-11-26 22:36 ` [PATCH v2] " Heidi Fahim

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git