Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2242185
grep (3.8) warnings when running the unit tests or running "kdumpctl reset-crashkernel" on >= F39, # unit tests Examples: 1) kdumpctl _find_kernel_path_by_release() returns the kernel path for the given release When call _find_kernel_path_by_release vmlinuz-6.2.11-200.fc37.x86_64
1.1) WARNING: There was output to stderr but not found expectation
stderr: grep: warning: stray \ before /
# spec/kdumpctl_general_spec.sh:169-172
# kdumpctl reset-crashkernel grep: warning: stray \ before / kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-6.6.8-200.fc39.x86_64. Please reboot the system for the change to take effect.
This warning can be reproduced by echo 'kernel="/boot/vmlinuz-6.4.6-200.fc38.x86_64"' | grep -E "^kernel=.*$_release(/\w+)?"$"
This patch removes unneeded backslash. It also adds a test for systemd-boot path. And for simplification, Parameters:dynamic is now used to generate test data dynamically.
Fixes: 8af05dc4 ("kdumpctl: Add support for systemd-boot paths") Signed-off-by: Coiby Xu coxu@redhat.com --- kdumpctl | 2 +- spec/kdumpctl_general_spec.sh | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 45277a79..30eb27dd 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1412,7 +1412,7 @@ _find_kernel_path_by_release()
# Insert '/' before '+' to cope with grep's EREs _release=${_release//+/\+} - _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$") + _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$") _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str") if [[ -z $_kernel_path ]]; then ddebug "kernel $_release doesn't exist" diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index 243aeca3..f69924f6 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -156,16 +156,25 @@ Describe 'kdumpctl' End
Describe '_find_kernel_path_by_release()' + # When the array length changes, the Parameters:dynamic should change as well + kernel_paths=(/boot/vmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-322.el9.aarch64 /boot/efi/36b54597c46383/6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64/linux) + kernels=(vmlinuz-6.2.11-200.fc37.x86_64 vmlinuz-5.14.0-316.el9.aarch64+64k vmlinuz-5.14.0-322.el9.aarch64 6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64) + grubby() { - echo -e 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"' + for key in "${!kernel_paths[@]}"; do + echo "kernel="${kernel_paths[$key]}"" + done }
- Parameters - # parameter answer - vmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-6.2.11-200.fc37.x86_64 - vmlinuz-5.14.0-322.el9.aarch64 /boot/vmlinuz-5.14.0-322.el9.aarch64 - vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-316.el9.aarch64+64k + Parameters:dynamic + # Due to a bug [1] in shellspec, hardcode the loop number instead of using the + # array length + # [1] https://github.com/shellspec/shellspec/issues/259 + for key in {0..3}; do + %data "${kernels[$key]}" "${kernel_paths[$key]}" + done End + It 'returns the kernel path for the given release' When call _find_kernel_path_by_release "$1" The output should equal "$2"
Hi Coiby,
On Tue, 30 Jan 2024 14:39:38 +0800 Coiby Xu coxu@redhat.com wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2242185
grep (3.8) warnings when running the unit tests or running "kdumpctl reset-crashkernel" on >= F39, # unit tests Examples: 1) kdumpctl _find_kernel_path_by_release() returns the kernel path for the given release When call _find_kernel_path_by_release vmlinuz-6.2.11-200.fc37.x86_64
1.1) WARNING: There was output to stderr but not found expectation stderr: grep: warning: stray \ before / # spec/kdumpctl_general_spec.sh:169-172 # kdumpctl reset-crashkernel grep: warning: stray \ before / kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-6.6.8-200.fc39.x86_64. Please reboot the system for the change to take effect.This warning can be reproduced by echo 'kernel="/boot/vmlinuz-6.4.6-200.fc38.x86_64"' | grep -E "^kernel=.*$_release(/\w+)?"$"
This patch removes unneeded backslash. It also adds a test for systemd-boot path. And for simplification, Parameters:dynamic is now used to generate test data dynamically.
Fixes: 8af05dc4 ("kdumpctl: Add support for systemd-boot paths") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 2 +- spec/kdumpctl_general_spec.sh | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 45277a79..30eb27dd 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1412,7 +1412,7 @@ _find_kernel_path_by_release()
# Insert '/' before '+' to cope with grep's EREs _release=${_release//+/\+}
- _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$")
- _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$") _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str") if [[ -z $_kernel_path ]]; then ddebug "kernel $_release doesn't exist"
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index 243aeca3..f69924f6 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -156,16 +156,25 @@ Describe 'kdumpctl' End
Describe '_find_kernel_path_by_release()'
# When the array length changes, the Parameters:dynamic should change as wellkernel_paths=(/boot/vmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-322.el9.aarch64 /boot/efi/36b54597c46383/6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64/linux)kernels=(vmlinuz-6.2.11-200.fc37.x86_64 vmlinuz-5.14.0-316.el9.aarch64+64k vmlinuz-5.14.0-322.el9.aarch64 6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64)
all in all the patch looks good. Only the two lines above are extremely hard to read because they are so long. Could you change formatting so every line only contains a single kernel. I believe that will improve readability a lot.
Besides this tiny nit Reviewed-by: Philipp Rudo prudo@redhat.com
grubby() {
echo -e 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"'
for key in "${!kernel_paths[@]}"; doecho "kernel=\"${kernel_paths[$key]}\""done
Parameters# parameter answervmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-6.2.11-200.fc37.x86_64vmlinuz-5.14.0-322.el9.aarch64 /boot/vmlinuz-5.14.0-322.el9.aarch64vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-316.el9.aarch64+64k
Parameters:dynamic# Due to a bug [1] in shellspec, hardcode the loop number instead of using the# array length# [1] https://github.com/shellspec/shellspec/issues/259for key in {0..3}; do%data "${kernels[$key]}" "${kernel_paths[$key]}"done- It 'returns the kernel path for the given release' When call _find_kernel_path_by_release "$1" The output should equal "$2"
On Tue, Jan 30, 2024 at 10:04:06AM +0100, Philipp Rudo wrote:
Hi Coiby,
On Tue, 30 Jan 2024 14:39:38 +0800 Coiby Xu coxu@redhat.com wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2242185
grep (3.8) warnings when running the unit tests or running "kdumpctl reset-crashkernel" on >= F39, # unit tests Examples: 1) kdumpctl _find_kernel_path_by_release() returns the kernel path for the given release When call _find_kernel_path_by_release vmlinuz-6.2.11-200.fc37.x86_64
1.1) WARNING: There was output to stderr but not found expectation stderr: grep: warning: stray \ before / # spec/kdumpctl_general_spec.sh:169-172 # kdumpctl reset-crashkernel grep: warning: stray \ before / kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-6.6.8-200.fc39.x86_64. Please reboot the system for the change to take effect.This warning can be reproduced by echo 'kernel="/boot/vmlinuz-6.4.6-200.fc38.x86_64"' | grep -E "^kernel=.*$_release(/\w+)?"$"
This patch removes unneeded backslash. It also adds a test for systemd-boot path. And for simplification, Parameters:dynamic is now used to generate test data dynamically.
Fixes: 8af05dc4 ("kdumpctl: Add support for systemd-boot paths") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 2 +- spec/kdumpctl_general_spec.sh | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 45277a79..30eb27dd 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1412,7 +1412,7 @@ _find_kernel_path_by_release()
# Insert '/' before '+' to cope with grep's EREs _release=${_release//+/\+}
- _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$")
- _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$") _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str") if [[ -z $_kernel_path ]]; then ddebug "kernel $_release doesn't exist"
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index 243aeca3..f69924f6 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -156,16 +156,25 @@ Describe 'kdumpctl' End
Describe '_find_kernel_path_by_release()'
# When the array length changes, the Parameters:dynamic should change as wellkernel_paths=(/boot/vmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-322.el9.aarch64 /boot/efi/36b54597c46383/6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64/linux)kernels=(vmlinuz-6.2.11-200.fc37.x86_64 vmlinuz-5.14.0-316.el9.aarch64+64k vmlinuz-5.14.0-322.el9.aarch64 6.4.0-0.rc0.20230427git6e98b09da931.5.fc39.aarch64)all in all the patch looks good. Only the two lines above are extremely hard to read because they are so long. Could you change formatting so every line only contains a single kernel. I believe that will improve readability a lot.
Besides this tiny nit Reviewed-by: Philipp Rudo prudo@redhat.com
Patch merged with the suggestion of formatting the code applied, thanks!
-
Coiby Xu -
Philipp Rudo