[PATCH 0/2] Make it visible that not all tests can be run
Inspired by: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/TSKY... Martin Kletzander (2): build: Split optional programs into test and rest build: Let users know not all tests might run meson.build | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) -- 2.43.1
To be used in the following commit. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- meson.build | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/meson.build b/meson.build index dbd9cad6df5a..2f3d73b93085 100644 --- a/meson.build +++ b/meson.build @@ -795,12 +795,17 @@ endforeach # optional programs -optional_programs = [ +optional_test_programs = [ 'augparse', 'black', + 'flake8', + 'pdwtags', + 'pytest', +] + +optional_programs = [ 'dmidecode', 'ebtables', - 'flake8', 'ip', 'ip6tables', 'iptables', @@ -809,11 +814,9 @@ optional_programs = [ 'mm-ctl', 'modprobe', 'ovs-vsctl', - 'pdwtags', - 'pytest', 'rmmod', 'tc', -] +] + optional_test_programs foreach name : optional_programs prog = find_program(name, required: false, dirs: libvirt_sbin_path) -- 2.43.1
We warned users before the meson times, so do like an S Club 7 and bring it all back. Add the information into the 'Miscellaneous' section of the summary, because even though using `warning()` looks better, it scrolls on by once the summary is printed. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- meson.build | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/meson.build b/meson.build index 2f3d73b93085..6482493d7136 100644 --- a/meson.build +++ b/meson.build @@ -818,6 +818,7 @@ optional_programs = [ 'tc', ] + optional_test_programs +missing_optional_programs = [] foreach name : optional_programs prog = find_program(name, required: false, dirs: libvirt_sbin_path) varname = name.underscorify() @@ -825,6 +826,9 @@ foreach name : optional_programs prog_path = prog.full_path() else prog_path = name + if name in optional_test_programs + missing_optional_programs += [ name ] + endif endif conf.set_quoted(varname.to_upper(), prog_path) @@ -2330,6 +2334,10 @@ misc_summary = { 'sysctl config': conf.has('WITH_SYSCTL'), 'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'), } +if missing_optional_programs.length() > 0 + misc_summary += {'Some programs are missing, not all tests will be executed': + missing_optional_programs} +endif summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ') devtools_summary = { -- 2.43.1
On Mon, Feb 19, 2024 at 10:35:14AM +0100, Martin Kletzander wrote:
We warned users before the meson times, so do like an S Club 7 and bring it all back.
Add the information into the 'Miscellaneous' section of the summary, because even though using `warning()` looks better, it scrolls on by once the summary is printed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- meson.build | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/meson.build b/meson.build index 2f3d73b93085..6482493d7136 100644 --- a/meson.build +++ b/meson.build @@ -818,6 +818,7 @@ optional_programs = [ 'tc', ] + optional_test_programs
+missing_optional_programs = [] foreach name : optional_programs prog = find_program(name, required: false, dirs: libvirt_sbin_path) varname = name.underscorify() @@ -825,6 +826,9 @@ foreach name : optional_programs prog_path = prog.full_path() else prog_path = name + if name in optional_test_programs + missing_optional_programs += [ name ] + endif endif
conf.set_quoted(varname.to_upper(), prog_path) @@ -2330,6 +2334,10 @@ misc_summary = { 'sysctl config': conf.has('WITH_SYSCTL'), 'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'), } +if missing_optional_programs.length() > 0 + misc_summary += {'Some programs are missing, not all tests will be executed': + missing_optional_programs} +endif summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ')
devtools_summary = { -- 2.43.1 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
I like it, but I'm going to suggest a slightly tweaked implementation. With the diff below squashed in, the output will turn into Optional programs Missing : black (some tests will be skipped!) which is less busy and more readable IMO. I think it's more likely to catch the user's eye compared to being yet another line at the bottom of the Miscellaneous section. diff --git a/meson.build b/meson.build index 6482493d71..7845f60ff7 100644 --- a/meson.build +++ b/meson.build @@ -2334,10 +2334,6 @@ misc_summary = { 'sysctl config': conf.has('WITH_SYSCTL'), 'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'), } -if missing_optional_programs.length() > 0 - misc_summary += {'Some programs are missing, not all tests will be executed': - missing_optional_programs} -endif summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ') devtools_summary = { @@ -2345,6 +2341,15 @@ devtools_summary = { } summary(devtools_summary, section: 'Developer Tools', bool_yn: true) +if missing_optional_programs.length() > 0 + missing_list = ' '.join(missing_optional_programs) + missing_warn = ' (some tests will be skipped!)' + test_programs_summary = { + 'Missing': missing_list + missing_warn, + } + summary(test_programs_summary, section: 'Optional programs', bool_yn: true) +endif + if conf.has('WITH_QEMU') qemu_warn = '' if qemu_user == 'root' -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Feb 19, 2024 at 04:47:52AM -0800, Andrea Bolognani wrote:
On Mon, Feb 19, 2024 at 10:35:14AM +0100, Martin Kletzander wrote:
We warned users before the meson times, so do like an S Club 7 and bring it all back.
Add the information into the 'Miscellaneous' section of the summary, because even though using `warning()` looks better, it scrolls on by once the summary is printed.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- meson.build | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/meson.build b/meson.build index 2f3d73b93085..6482493d7136 100644 --- a/meson.build +++ b/meson.build @@ -818,6 +818,7 @@ optional_programs = [ 'tc', ] + optional_test_programs
+missing_optional_programs = [] foreach name : optional_programs prog = find_program(name, required: false, dirs: libvirt_sbin_path) varname = name.underscorify() @@ -825,6 +826,9 @@ foreach name : optional_programs prog_path = prog.full_path() else prog_path = name + if name in optional_test_programs + missing_optional_programs += [ name ] + endif endif
conf.set_quoted(varname.to_upper(), prog_path) @@ -2330,6 +2334,10 @@ misc_summary = { 'sysctl config': conf.has('WITH_SYSCTL'), 'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'), } +if missing_optional_programs.length() > 0 + misc_summary += {'Some programs are missing, not all tests will be executed': + missing_optional_programs} +endif summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ')
devtools_summary = { -- 2.43.1 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
I like it, but I'm going to suggest a slightly tweaked implementation.
With the diff below squashed in, the output will turn into
Optional programs Missing : black (some tests will be skipped!)
which is less busy and more readable IMO. I think it's more likely to catch the user's eye compared to being yet another line at the bottom of the Miscellaneous section.
I liked that the first column got wider, but to be honest it catches your eye only if you are used to the output. And I like your version better. If you have it squashed locally, then feel free to push that.
diff --git a/meson.build b/meson.build index 6482493d71..7845f60ff7 100644 --- a/meson.build +++ b/meson.build @@ -2334,10 +2334,6 @@ misc_summary = { 'sysctl config': conf.has('WITH_SYSCTL'), 'userfaultfd sysctl': conf.has('WITH_USERFAULTFD_SYSCTL'), } -if missing_optional_programs.length() > 0 - misc_summary += {'Some programs are missing, not all tests will be executed': - missing_optional_programs} -endif summary(misc_summary, section: 'Miscellaneous', bool_yn: true, list_sep: ' ')
devtools_summary = { @@ -2345,6 +2341,15 @@ devtools_summary = { } summary(devtools_summary, section: 'Developer Tools', bool_yn: true)
+if missing_optional_programs.length() > 0 + missing_list = ' '.join(missing_optional_programs) + missing_warn = ' (some tests will be skipped!)' + test_programs_summary = { + 'Missing': missing_list + missing_warn, + } + summary(test_programs_summary, section: 'Optional programs', bool_yn: true)
the bool_yn doesn't do anything here, but that's fine Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
+endif + if conf.has('WITH_QEMU') qemu_warn = '' if qemu_user == 'root' -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Feb 19, 2024 at 03:01:29PM +0100, Martin Kletzander wrote:
On Mon, Feb 19, 2024 at 04:47:52AM -0800, Andrea Bolognani wrote:
On Mon, Feb 19, 2024 at 10:35:14AM +0100, Martin Kletzander wrote:
+if missing_optional_programs.length() > 0 + misc_summary += {'Some programs are missing, not all tests will be executed': + missing_optional_programs} +endif
I like it, but I'm going to suggest a slightly tweaked implementation.
With the diff below squashed in, the output will turn into
Optional programs Missing : black (some tests will be skipped!)
which is less busy and more readable IMO. I think it's more likely to catch the user's eye compared to being yet another line at the bottom of the Miscellaneous section.
I liked that the first column got wider, but to be honest it catches your eye only if you are used to the output.
And I like your version better. If you have it squashed locally, then feel free to push that.
I think it's probably easier if you just squash it in yourself. You can also add my Reviewed-by: Andrea Bolognani <abologna@redhat.com> to both patches when you do that. Maybe give Michal a chance to object to the change before pushing, just in case it would make him retract his R-b.
+ summary(test_programs_summary, section: 'Optional programs', bool_yn: true)
the bool_yn doesn't do anything here, but that's fine
Yeah, I copied it from the existing invocations but you're right that it's unnecessary here. By all means drop it. -- Andrea Bolognani / Red Hat / Virtualization
On 2/19/24 10:35, Martin Kletzander wrote:
Inspired by:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/TSKY...
Martin Kletzander (2):
build: Split optional programs into test and rest
build: Let users know not all tests might run
meson.build | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Andrea Bolognani -
Martin Kletzander -
Michal Prívozník