v1: - issues found by Tao - clean up files creatd by mktemp - use DUMMY_PARAM when calling a mocked - clean up unsued parameters - mock uname for kdump mode tests to fake current running kernel - {add,remove}_kernel_arg_in_grub_etc_default replaced by _update_*
Potential benefits brought by unit tests =======================================
- catch as many bugs as possible and catch them as early as possible - improve the code quality and make the code robust - Keep API stable - prevent regressions from change during the development and in the future (it's quite annoying to solve one problem only to find another cropping up) - make it easy for code review because of concrete test cases
Why ShellSpec =============
ShellSpec [1] is chosen for the following benefits, - rich features: to name a few - mocking - Parameterized tests - code coverage - good documentation and examples
For the full comparison between shellspec and other frameworks, please refer to [2].
How to run the tests ====================
After installing shellspec and optionally kcov $ curl -fsSL https://git.io/shellspec | sh $ sudo dnf install kcov
1. go to the root of this repo 2. create a file with _spec as suffix in the spec folder, e.g. spec/kdumpctl_spec.sh 3. run shellspec shellspec --kcov --kcov-options "--include-pattern=kdumpctl,kdump-lib.sh,kdump-lib-initramfs.sh,kdump-logger.sh"
Running: /bin/sh [bash 5.1.0(1)-release] ................................................................
Finished in 11.15 seconds (user 6.97 seconds, sys 9.54 seconds) 64 examples, 0 failures
Code covered: 16.08%, Executed lines: 224, Instrumented lines: 1393
[1] https://github.com/shellspec/shellspec [2] https://github.com/dodie/testing-in-bash#detailed-comparision
Coiby Xu (8): unit tests: prepare for kdumpctl and kdump-lib.sh to be unit-tested unit tests: add tests for get_grub_kernel_boot_parameter unit tests: add tests for get_dump_mode_by_fadump_val unit tests: add tests for kdumpctl read_proc_environ_var and _is_osbuild unit tests: add tests for _{update,read}_kernel_arg_in_grub_etc_default in kdumpctl unit tests: add tests for "kdumpctl reset-crashkernel" unit tests: add tests for kdump_get_conf_val in kdump-lib-initramfs.sh unit tests: add check_config with with the default kdump.conf
.shellspec | 0 kdump-lib.sh | 7 +- kdumpctl | 24 +- spec/kdump-lib-initramfs_spec.sh | 41 ++++ spec/kdumpctl_general_spec.sh | 183 ++++++++++++++ spec/kdumpctl_reset_crashkernel_spec.sh | 223 ++++++++++++++++++ spec/support/bin/@grubby | 3 + ...846f63134c7295458cf36300ba5b-0-rescue.conf | 8 + ...58cf36300ba5b-5.14.14-200.fc34.x86_64.conf | 8 + ...458cf36300ba5b-5.15.6-100.fc34.x86_64.conf | 8 + spec/support/grub_env | 3 + 11 files changed, 499 insertions(+), 9 deletions(-) create mode 100644 .shellspec create mode 100644 spec/kdump-lib-initramfs_spec.sh create mode 100644 spec/kdumpctl_general_spec.sh create mode 100644 spec/kdumpctl_reset_crashkernel_spec.sh create mode 100755 spec/support/bin/@grubby create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf create mode 100644 spec/support/grub_env
Currently there are two issues with unit-testing the functions defined in kdumpctl and other shell scripts after sourcing them, - kdumpctl would call main which requires root permission and would create single instance lock (/var/lock/kdump) - kdumpctl and other shell scripts directly source files under /usr/lib/kdump/
When ShellSpec load a script via "Include", it defines the__SOURCED__ variable. By making use of __SOURCED__, we can 1. let kdumpctl not call main when kdumpctl is "Include"d by ShellSpec 2. instruct kdumpctl and kdump-lib.sh to source the files in the repo when running ShelSpec tests
Signed-off-by: Coiby Xu coxu@redhat.com --- kdump-lib.sh | 7 +++++-- kdumpctl | 24 +++++++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/kdump-lib.sh b/kdump-lib.sh index 4ed5035..50b69e1 100755 --- a/kdump-lib.sh +++ b/kdump-lib.sh @@ -2,8 +2,11 @@ # # Kdump common variables and functions # - -. /usr/lib/kdump/kdump-lib-initramfs.sh +if [[ ${__SOURCED__:+x} ]]; then + . ./kdump-lib-initramfs.sh +else + . /lib/kdump/kdump-lib-initramfs.sh +fi
FADUMP_ENABLED_SYS_NODE="/sys/kernel/fadump_enabled"
diff --git a/kdumpctl b/kdumpctl index ac6ffeb..c7b821f 100755 --- a/kdumpctl +++ b/kdumpctl @@ -33,8 +33,14 @@ fi
[[ $dracutbasedir ]] || dracutbasedir=/usr/lib/dracut . $dracutbasedir/dracut-functions.sh -. /lib/kdump/kdump-lib.sh -. /lib/kdump/kdump-logger.sh + +if [[ ${__SOURCED__:+x} ]]; then + KDUMP_LIB_PATH=. +else + KDUMP_LIB_PATH=/lib/kdump +fi +. $KDUMP_LIB_PATH/kdump-lib.sh +. $KDUMP_LIB_PATH/kdump-logger.sh
#initiate the kdump logger if ! dlog_init; then @@ -1699,11 +1705,6 @@ reset_crashkernel_for_installed_kernel() fi }
-if [[ ! -f $KDUMP_CONFIG_FILE ]]; then - derror "Error: No kdump config file found!" - exit 1 -fi - main() { # Determine if the dump mode is kdump or fadump @@ -1776,6 +1777,15 @@ main() esac }
+if [[ ${__SOURCED__:+x} ]]; then + return +fi + +if [[ ! -f $KDUMP_CONFIG_FILE ]]; then + derror "Error: No kdump config file found!" + exit 1 +fi + # Other kdumpctl instances will block in queue, until this one exits single_instance_lock
This test suite makes use of three features provided by ShellSpec - funcion-based mock [2]: mock a function by re-defining and exporting it - parameterized tests [3]: run multiple sets of input against the same test - %text directive [4]: similar to heredoc but free of the indentation issue
Note 1. Describe and Context are aliases for ExampleGroup which a block for grouping example groups or examples [5]. Describe and Context are used to improve readability. 2. ShellSpec requires .shellspec file.
[1] https://github.com/dodie/testing-in-bash#detailed-comparision [2] https://github.com/shellspec/shellspec#function-based-mock [3] https://github.com/shellspec/shellspec#parameters---parameterized-example [4] https://github.com/shellspec/shellspec#text---embedded-text [5] https://github.com/shellspec/shellspec#dsl-syntax
Signed-off-by: Coiby Xu coxu@redhat.com --- .shellspec | 0 spec/kdumpctl_general_spec.sh | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 .shellspec create mode 100644 spec/kdumpctl_general_spec.sh
diff --git a/.shellspec b/.shellspec new file mode 100644 index 0000000..e69de29 diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh new file mode 100644 index 0000000..e1d8698 --- /dev/null +++ b/spec/kdumpctl_general_spec.sh @@ -0,0 +1,48 @@ +#!/bin/bash +Describe 'kdumpctl' + Include ./kdumpctl + + Describe 'get_grub_kernel_boot_parameter()' + grubby() { + %text + #|index=1 + #|kernel="/boot/vmlinuz-5.14.14-200.fc34.x86_64" + #|args="crashkernel=11M nvidia-drm.modeset=1 crashkernel=100M ro rhgb quiet crcrashkernel=200M crashkernel=32T-64T:128G,64T-102400T:180G fadump=on" + #|root="UUID=45fdf703-3966-401b-b8f7-cf056affd2b0" + } + DUMMY_PARAM=/boot/vmlinuz + + Context "when given a kernel parameter in different positions" + # Test the following cases: + # - the kernel parameter in the end + # - the kernel parameter in the first + # - the kernel parameter is crashkernel (suffix of crcrashkernel) + # - the kernel parameter that does not exist + # - the kernel parameter doesn't have a value + Parameters + # parameter answer + fadump on + nvidia-drm.modeset 1 + crashkernel 32T-64T:128G,64T-102400T:180G + aaaa "" + ro "" + End + + It 'should retrieve the value succesfully' + When call get_grub_kernel_boot_parameter "$DUMMY_PARAM" "$2" + The output should equal "$3" + End + End + + It 'should retrive the last value if multiple <parameter=value> entries exist' + When call get_grub_kernel_boot_parameter "$DUMMY_PARAM" crashkernel + The output should equal '32T-64T:128G,64T-102400T:180G' + End + + It 'should fail when called with kernel_path=ALL' + When call get_grub_kernel_boot_parameter ALL ro + The status should be failure + The error should include "kernel_path=ALL invalid" + End + End +End
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdumpctl_general_spec.sh | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index e1d8698..b968f74 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -45,4 +45,30 @@ Describe 'kdumpctl' The error should include "kernel_path=ALL invalid" End End + + Describe 'get_dump_mode_by_fadump_val()' + + Context 'when given valid fadump values' + Parameters + "#1" on fadump + "#2" nocma fadump + "#3" "" kdump + "#4" off kdump + End + It "should return the dump mode correctly" + When call get_dump_mode_by_fadump_val "$2" + The output should equal "$3" + The status should be success + End + End + + It 'should complain given invalid fadump value' + When call get_dump_mode_by_fadump_val /boot/vmlinuz + The status should be failure + The error should include 'invalid fadump' + End + + End + + End
AfterAll is an example group hook [1] which would be run after the group tests are executed. Use this hook to clean up the files created by mktemp.
[1] https://github.com/shellspec/shellspec#beforeall-afterall---example-group-ho...
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdumpctl_general_spec.sh | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index b968f74..fd449d6 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -70,5 +70,39 @@ Describe 'kdumpctl'
End
+ Describe "read_proc_environ_var()" + environ_test_file=$(mktemp -t spec_test_environ_test_file.XXXXXXXXXX) + cleanup() { + rm -rf $environ_test_file + } + AfterAll 'cleanup' + echo -ne "container=bwrap-osbuild\x00SSH_AUTH_SOCK=/tmp/ssh-XXXXXXEbw33A/agent.1794\x00SSH_AGENT_PID=1929\x00env=test_env" >$environ_test_file + Parameters + container bwrap-osbuild + SSH_AUTH_SOCK /tmp/ssh-XXXXXXEbw33A/agent.1794 + env test_env + not_exist "" + End + It 'should read the environ variable value as expected' + When call read_proc_environ_var "$1" "$environ_test_file" + The output should equal "$2" + The status should be success + End + End + + Describe "_is_osbuild()" + environ_test_file=$(mktemp -t spec_test_environ_test_file.XXXXXXXXXX) + _OSBUILD_ENVIRON_PATH="$environ_test_file" + Parameters + 'container=bwrap-osbuild' success + '' failure + End + It 'should be able to tell if it is the osbuild environment' + echo -ne "$1" >$environ_test_file + When call _is_osbuild + The status should be $2 + The stderr should equal "" + End + End
End
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdumpctl_general_spec.sh | 67 +++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+)
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index fd449d6..e7d057e 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -105,4 +105,71 @@ Describe 'kdumpctl' End End
+ Describe '_update_kernel_arg_in_grub_etc_default()' + GRUB_ETC_DEFAULT=/tmp/default_grub + + cleanup() { + rm -rf "$GRUB_ETC_DEFAULT" + } + AfterAll 'cleanup' + + Context 'when the given parameter is in different positions' + Parameters + "crashkernel=222M fadump=on rhgb quiet" crashkernel 333M + " fadump=on crashkernel=222M rhgb quiet" crashkernel 333M + "fadump=on rhgb quiet crashkernel=222M" crashkernel 333M + "fadump=on rhgb quiet" crashkernel 333M + "fadump=on foo=bar1 rhgb quiet" foo bar2 + End + + It 'should update the kernel parameter correctly' + echo 'GRUB_CMDLINE_LINUX="'"$1"'"' >$GRUB_ETC_DEFAULT + When call _update_kernel_arg_in_grub_etc_default "$2" "$3" + # the updated kernel parameter should appear in the end + The contents of file $GRUB_ETC_DEFAULT should include "$2=$3"" + End + End + + It 'should only update the given parameter and not update the parameter that has the given parameter as suffix' + echo 'GRUB_CMDLINE_LINUX="fadump=on rhgb quiet ckcrashkernel=222M"' >$GRUB_ETC_DEFAULT + _ck_val=1G-4G:192M,4G-64G:256M,64G-102400T:512M + When call _update_kernel_arg_in_grub_etc_default crashkernel "$_ck_val" + The contents of file $GRUB_ETC_DEFAULT should include "crashkernel=$_ck_val"" + The contents of file $GRUB_ETC_DEFAULT should include "ckcrashkernel=222M" + End + + It 'should be able to handle the cases of there are multiple crashkernel entries' + echo 'GRUB_CMDLINE_LINUX="fadump=on rhgb quiet crashkernel=101M crashkernel=222M"' >$GRUB_ETC_DEFAULT + _ck_val=1G-4G:192M,4G-64G:256M,64G-102400T:512M + When call _update_kernel_arg_in_grub_etc_default crashkernel "$_ck_val" + The contents of file $GRUB_ETC_DEFAULT should include "crashkernel=$_ck_val"" + The contents of file $GRUB_ETC_DEFAULT should not include "crashkernel=222M" + End + + Context 'when it removes a kernel parameter' + + It 'should remove all values for given arg' + echo 'GRUB_CMDLINE_LINUX="crashkernel=33M crashkernel=11M fadump=on crashkernel=222M"' >$GRUB_ETC_DEFAULT + When call _update_kernel_arg_in_grub_etc_default crashkernel + The contents of file $GRUB_ETC_DEFAULT should equal 'GRUB_CMDLINE_LINUX="fadump=on"' + End + + It 'should not remove args that have the given arg as suffix' + echo 'GRUB_CMDLINE_LINUX="ckcrashkernel=33M crashkernel=11M ckcrashkernel=222M"' >$GRUB_ETC_DEFAULT + When call _update_kernel_arg_in_grub_etc_default crashkernel + The contents of file $GRUB_ETC_DEFAULT should equal 'GRUB_CMDLINE_LINUX="ckcrashkernel=33M ckcrashkernel=222M"' + End + End + + End + + Describe '_read_kernel_arg_in_grub_etc_default()' + GRUB_ETC_DEFAULT=/tmp/default_grub + It 'should read the value for given arg' + echo 'GRUB_CMDLINE_LINUX="crashkernel=33M crashkernel=11M ckcrashkernel=222M"' >$GRUB_ETC_DEFAULT + When call _read_kernel_arg_in_grub_etc_default crashkernel + The output should equal '11M' + End + End + End
This commit adds a relatively thorough test suite for kdumpctl reset-crashkernel [--fadump=[on|off|nocma]] [--kernel=path_to_kernel] [--reboot] as implemented in commit 140da74 ("rewrite reset_crashkernel to support fadump and to used by RPM scriptlet").
grubby have a few options to support its own testing, - --no-etc-grub-update, not update /etc/default/grub - --bad-image-okay, don't check the validity of the image - --env, specify custom grub2 environment block file to avoid modifying the default /boot/grub2/grubenv - --bls-directory, specify custom BootLoaderSpec config files to avoid modifying the default /boot/loader/entries
So the grubby called by kdumpctl is mocked as @grubby --grub2 --no-etc-grub-update --bad-image-okay --env=$SPEC_TEST_DIR/env_temp -b $SPEC_TEST_DIR/boot_load_entries "$@" in the tests. To be able to call the actual grubby in the mock function [1], ShellSpec provides the following command $ shellspec --gen-bin @grubby to generate spec/support/bins/@grubby which is used to call the actual grubby.
kdumpctl has implemented its own version of updating /etc/default/grub in _update_kernel_cmdline_in_grub_etc_default. To avoiding writing to /etc/default/grub, this function is mocked as outputting its name and received arguments similar to python unitest's assert_called_with.
[1] https://github.com/shellspec/shellspec#execute-the-actual-command-within-a-m...
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdumpctl_reset_crashkernel_spec.sh | 223 ++++++++++++++++++ spec/support/bin/@grubby | 3 + ...846f63134c7295458cf36300ba5b-0-rescue.conf | 8 + ...58cf36300ba5b-5.14.14-200.fc34.x86_64.conf | 8 + ...458cf36300ba5b-5.15.6-100.fc34.x86_64.conf | 8 + spec/support/grub_env | 3 + 6 files changed, 253 insertions(+) create mode 100644 spec/kdumpctl_reset_crashkernel_spec.sh create mode 100755 spec/support/bin/@grubby create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf create mode 100644 spec/support/grub_env
diff --git a/spec/kdumpctl_reset_crashkernel_spec.sh b/spec/kdumpctl_reset_crashkernel_spec.sh new file mode 100644 index 0000000..3fc7459 --- /dev/null +++ b/spec/kdumpctl_reset_crashkernel_spec.sh @@ -0,0 +1,223 @@ +#!/bin/bash +Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]' + Include ./kdumpctl + kernel1=/boot/vmlinuz-5.15.6-100.fc34.x86_64 + kernel2=/boot/vmlinuz-5.14.14-200.fc34.x86_64 + ck=222M + KDUMP_SPEC_TEST_RUN_DIR=$(mktemp -d /tmp/spec_test.XXXXXXXXXX) + current_kernel=5.15.6-100.fc34.x86_64 + + setup() { + cp -r spec/support/boot_load_entries $KDUMP_SPEC_TEST_RUN_DIR + cp spec/support/grub_env $KDUMP_SPEC_TEST_RUN_DIR/env_temp + } + + cleanup() { + rm -rf $KDUMP_SPEC_TEST_RUN_DIR + } + + BeforeAll 'setup' + AfterAll 'cleanup' + + grubby() { + # - --no-etc-grub-update, not update /etc/default/grub + # - --bad-image-okay, don't check the validity of the image + # - --env, specify custom grub2 environment block file to avoid modifying + # the default /boot/grub2/grubenv + # - --bls-directory, specify custom BootLoaderSpec config files to avoid + # modifying the default /boot/loader/entries + @grubby --no-etc-grub-update --grub2 --bad-image-okay --env=$KDUMP_SPEC_TEST_RUN_DIR/env_temp -b $KDUMP_SPEC_TEST_RUN_DIR/boot_load_entries "$@" + } + + Describe "Test the kdump dump mode " + kdump_crashkernel=$(get_default_crashkernel kdump) + uname() { + if [[ $1 == '-m' ]]; then + echo -n x86_64 + elif [[ $1 == '-r' ]]; then + echo -n $current_kernel + fi + } + Context "when --kernel not specified" + grubby --args crashkernel=$ck --update-kernel ALL + Specify 'kdumpctl should warn the user that crashkernel has been udpated' + When call reset_crashkernel + The error should include "Updated crashkernel=$kdump_crashkernel" + End + + Specify 'Current running kernel should have crashkernel updated' + When call grubby --info $kernel1 + The line 3 of output should include crashkernel=$kdump_crashkernel + End + + Specify 'Other kernel still use the old crashkernel value' + When call grubby --info $kernel2 + The line 3 of output should include crashkernel=$ck + End + End + + Context "--kernel=ALL" + grubby --args crashkernel=$ck --update-kernel ALL + Specify 'kdumpctl should warn the user that crashkernel has been udpated' + When call reset_crashkernel --kernel=ALL + The error should include "Updated crashkernel=$kdump_crashkernel for kernel=$kernel1" + The error should include "Updated crashkernel=$kdump_crashkernel for kernel=$kernel2" + End + + Specify 'kernel1 should have crashkernel updated' + When call grubby --info $kernel1 + The line 3 of output should include crashkernel=$kdump_crashkernel + End + + Specify 'kernel2 should have crashkernel updated' + When call grubby --info $kernel2 + The line 3 of output should include crashkernel=$kdump_crashkernel + End + End + + Context "--kernel=/boot/one-kernel to update one specified kernel" + grubby --args crashkernel=$ck --update-kernel ALL + Specify 'kdumpctl should warn the user that crashkernel has been updated' + When call reset_crashkernel --kernel=$kernel1 + The error should include "Updated crashkernel=$kdump_crashkernel for kernel=$kernel1" + End + + Specify 'kernel1 should have crashkernel updated' + When call grubby --info $kernel1 + The line 3 of output should include crashkernel=$kdump_crashkernel + End + + Specify 'kernel2 should have the old crashkernel' + When call grubby --info $kernel2 + The line 3 of output should include crashkernel=$ck + End + + End + + End + + Describe "FADump" fadump + uname() { + if [[ $1 == '-m' ]]; then + echo -n ppc64le + elif [[ $1 == '-r' ]]; then + echo -n $current_kernel + fi + } + + _update_kernel_arg_in_grub_etc_default() { + # don't modify /etc/default/grub during the test + echo _update_kernel_arg_in_grub_etc_default "$@" + } + + kdump_crashkernel=$(get_default_crashkernel kdump) + fadump_crashkernel=$(get_default_crashkernel fadump) + Context "when no --kernel specified" + grubby --args crashkernel=$ck --update-kernel ALL + grubby --remove-args=fadump --update-kernel ALL + Specify 'kdumpctl should warn the user that crashkernel has been udpated' + When call reset_crashkernel + The error should include "Updated crashkernel=$kdump_crashkernel" + End + + Specify 'Current running kernel should have crashkernel updated' + When call grubby --info $kernel1 + The line 3 of output should include crashkernel=$kdump_crashkernel + End + + Specify 'Other kernel still use the old crashkernel value' + When call grubby --info $kernel2 + The line 3 of output should include crashkernel=$ck + End + End + + Context "--kernel=ALL --fadump=on" + grubby --args crashkernel=$ck --update-kernel ALL + Specify 'kdumpctl should warn the user that crashkernel has been udpated' + When call reset_crashkernel --kernel=ALL --fadump=on + The line 1 of output should include "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel" + The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1" + The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel2" + End + + Specify 'kernel1 should have crashkernel updated' + When call grubby --info $kernel1 + The line 3 of output should include crashkernel=$fadump_crashkernel + End + + Specify 'kernel2 should have crashkernel updated' + When call get_grub_kernel_boot_parameter $kernel2 crashkernel + The output should equal $fadump_crashkernel + End + End + + Context "--kernel=/boot/one-kernel to update one specified kernel" + grubby --args crashkernel=$ck --update-kernel ALL + grubby --args fadump=on --update-kernel $kernel1 + Specify 'kdumpctl should warn the user that crashkernel has been updated' + When call reset_crashkernel --kernel=$kernel1 + The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1" + End + + Specify 'kernel1 should have crashkernel updated' + When call grubby --info $kernel1 + The line 3 of output should include crashkernel=$fadump_crashkernel + End + + Specify 'kernel2 should have the old crashkernel' + When call get_grub_kernel_boot_parameter $kernel2 crashkernel + The output should equal $ck + End + End + + Context "Update all kernels but without --fadump specified" + grubby --args crashkernel=$ck --update-kernel ALL + grubby --args fadump=on --update-kernel $kernel1 + Specify 'kdumpctl should warn the user that crashkernel has been updated' + When call reset_crashkernel --kernel=$kernel1 + The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1" + End + + Specify 'kernel1 should have crashkernel updated' + When call get_grub_kernel_boot_parameter $kernel1 crashkernel + The output should equal $fadump_crashkernel + End + + Specify 'kernel2 should have the old crashkernel' + When call get_grub_kernel_boot_parameter $kernel2 crashkernel + The output should equal $ck + End + End + + Context 'Switch between fadump=on and fadump=nocma' + grubby --args crashkernel=$ck --update-kernel ALL + grubby --args fadump=on --update-kernel ALL + Specify 'fadump=on to fadump=nocma' + When call reset_crashkernel --kernel=ALL --fadump=nocma + The line 1 of output should equal "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel" + The line 2 of output should equal "_update_kernel_arg_in_grub_etc_default fadump nocma" + The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1" + The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel2" + End + + Specify 'kernel1 should have fadump=nocma in cmdline' + When call get_grub_kernel_boot_parameter $kernel1 fadump + The output should equal nocma + End + + Specify 'fadump=nocma to fadump=on' + When call reset_crashkernel --kernel=ALL --fadump=on + The line 1 of output should equal "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel" + The line 2 of output should equal "_update_kernel_arg_in_grub_etc_default fadump on" + The error should include "Updated fadump=on for kernel=$kernel1" + End + + Specify 'kernel2 should have fadump=on in cmdline' + When call get_grub_kernel_boot_parameter $kernel1 fadump + The output should equal on + End + + End + + End +End diff --git a/spec/support/bin/@grubby b/spec/support/bin/@grubby new file mode 100755 index 0000000..2a9b33f --- /dev/null +++ b/spec/support/bin/@grubby @@ -0,0 +1,3 @@ +#!/bin/sh -e +. "$SHELLSPEC_SUPPORT_BIN" +invoke grubby "$@" diff --git a/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf new file mode 100644 index 0000000..b821952 --- /dev/null +++ b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf @@ -0,0 +1,8 @@ +title Fedora (0-rescue-e986846f63134c7295458cf36300ba5b) 33 (Workstation Edition) +version 0-rescue-e986846f63134c7295458cf36300ba5b +linux /boot/vmlinuz-0-rescue-e986846f63134c7295458cf36300ba5b +initrd /boot/initramfs-0-rescue-e986846f63134c7295458cf36300ba5b.img +options root=UUID=45fdf703-3966-401b-b8f7-cf056affd2b0 ro rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 crashkernel=4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-102400T:180G fadump=on +grub_users $grub_users +grub_arg --unrestricted +grub_class kernel diff --git a/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf new file mode 100644 index 0000000..08bd411 --- /dev/null +++ b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf @@ -0,0 +1,8 @@ +title Fedora (5.14.14-200.fc34.x86_64) 34 (Workstation Edition) +version 5.14.14-200.fc34.x86_64 +linux /boot/vmlinuz-5.14.14-200.fc34.x86_64 +initrd /boot/initramfs-5.14.14-200.fc34.x86_64.img +options root=UUID=45fdf703-3966-401b-b8f7-cf056affd2b0 ro rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 crashkernel=4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-102400T:180G fadump=on +grub_users $grub_users +grub_arg --unrestricted +grub_class kernel diff --git a/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf new file mode 100644 index 0000000..9259b99 --- /dev/null +++ b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf @@ -0,0 +1,8 @@ +title Fedora (5.15.6-100.fc34.x86_64) 34 (Workstation Edition) +version 5.15.6-100.fc34.x86_64 +linux /boot/vmlinuz-5.15.6-100.fc34.x86_64 +initrd /boot/initramfs-5.15.6-100.fc34.x86_64.img +options root=UUID=45fdf703-3966-401b-b8f7-cf056affd2b0 ro rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 crashkernel=4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-102400T:180G fadump=on +grub_users $grub_users +grub_arg --unrestricted +grub_class fedora diff --git a/spec/support/grub_env b/spec/support/grub_env new file mode 100644 index 0000000..a77303c --- /dev/null +++ b/spec/support/grub_env @@ -0,0 +1,3 @@ +# GRUB Environment Block +# WARNING: Do not edit this file by tools other than grub-editenv!!! +################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################## \ No newline at end of file
Hi Coiby,
On Tue, 1 Mar 2022 14:17:26 +0800 Coiby Xu coxu@redhat.com wrote:
This commit adds a relatively thorough test suite for kdumpctl reset-crashkernel [--fadump=[on|off|nocma]] [--kernel=path_to_kernel] [--reboot] as implemented in commit 140da74 ("rewrite reset_crashkernel to support fadump and to used by RPM scriptlet").
grubby have a few options to support its own testing,
- --no-etc-grub-update, not update /etc/default/grub
- --bad-image-okay, don't check the validity of the image
- --env, specify custom grub2 environment block file to avoid modifying the default /boot/grub2/grubenv
- --bls-directory, specify custom BootLoaderSpec config files to avoid modifying the default /boot/loader/entries
So the grubby called by kdumpctl is mocked as @grubby --grub2 --no-etc-grub-update --bad-image-okay --env=$SPEC_TEST_DIR/env_temp -b $SPEC_TEST_DIR/boot_load_entries "$@" in the tests. To be able to call the actual grubby in the mock function [1], ShellSpec provides the following command $ shellspec --gen-bin @grubby to generate spec/support/bins/@grubby which is used to call the actual grubby.
kdumpctl has implemented its own version of updating /etc/default/grub in _update_kernel_cmdline_in_grub_etc_default. To avoiding writing to /etc/default/grub, this function is mocked as outputting its name and received arguments similar to python unitest's assert_called_with.
[1] https://github.com/shellspec/shellspec#execute-the-actual-command-within-a-m...
Signed-off-by: Coiby Xu coxu@redhat.com
spec/kdumpctl_reset_crashkernel_spec.sh | 223 ++++++++++++++++++ spec/support/bin/@grubby | 3 + ...846f63134c7295458cf36300ba5b-0-rescue.conf | 8 + ...58cf36300ba5b-5.14.14-200.fc34.x86_64.conf | 8 + ...458cf36300ba5b-5.15.6-100.fc34.x86_64.conf | 8 + spec/support/grub_env | 3 + 6 files changed, 253 insertions(+) create mode 100644 spec/kdumpctl_reset_crashkernel_spec.sh create mode 100755 spec/support/bin/@grubby create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf create mode 100644 spec/support/grub_env
diff --git a/spec/kdumpctl_reset_crashkernel_spec.sh b/spec/kdumpctl_reset_crashkernel_spec.sh new file mode 100644 index 0000000..3fc7459 --- /dev/null +++ b/spec/kdumpctl_reset_crashkernel_spec.sh @@ -0,0 +1,223 @@ +#!/bin/bash +Describe 'kdumpctl reset-crashkernel [--kernel] [--fadump]'
- Include ./kdumpctl
- kernel1=/boot/vmlinuz-5.15.6-100.fc34.x86_64
- kernel2=/boot/vmlinuz-5.14.14-200.fc34.x86_64
- ck=222M
- KDUMP_SPEC_TEST_RUN_DIR=$(mktemp -d /tmp/spec_test.XXXXXXXXXX)
- current_kernel=5.15.6-100.fc34.x86_64
- setup() {
cp -r spec/support/boot_load_entries $KDUMP_SPEC_TEST_RUN_DIRcp spec/support/grub_env $KDUMP_SPEC_TEST_RUN_DIR/env_temp- }
- cleanup() {
rm -rf $KDUMP_SPEC_TEST_RUN_DIR- }
- BeforeAll 'setup'
- AfterAll 'cleanup'
- grubby() {
# - --no-etc-grub-update, not update /etc/default/grub# - --bad-image-okay, don't check the validity of the image# - --env, specify custom grub2 environment block file to avoid modifying# the default /boot/grub2/grubenv# - --bls-directory, specify custom BootLoaderSpec config files to avoid# modifying the default /boot/loader/entries@grubby --no-etc-grub-update --grub2 --bad-image-okay --env=$KDUMP_SPEC_TEST_RUN_DIR/env_temp -b $KDUMP_SPEC_TEST_RUN_DIR/boot_load_entries "$@"- }
- Describe "Test the kdump dump mode "
kdump_crashkernel=$(get_default_crashkernel kdump)
I think the line above should ...
uname() {if [[ $1 == '-m' ]]; thenecho -n x86_64elif [[ $1 == '-r' ]]; thenecho -n $current_kernelfi}
...go here. Otherwise you are not mocking the call to uname in kdump_get_arch_recommend_crashkernel.
Context "when --kernel not specified"grubby --args crashkernel=$ck --update-kernel ALLSpecify 'kdumpctl should warn the user that crashkernel has been udpated'When call reset_crashkernelThe error should include "Updated crashkernel=$kdump_crashkernel"EndSpecify 'Current running kernel should have crashkernel updated'When call grubby --info $kernel1The line 3 of output should include crashkernel=$kdump_crashkernel
Add this?
The line 3 of output should not include crashkernel=$ck
Thanks Philipp
EndSpecify 'Other kernel still use the old crashkernel value'When call grubby --info $kernel2The line 3 of output should include crashkernel=$ckEndEndContext "--kernel=ALL"grubby --args crashkernel=$ck --update-kernel ALLSpecify 'kdumpctl should warn the user that crashkernel has been udpated'When call reset_crashkernel --kernel=ALLThe error should include "Updated crashkernel=$kdump_crashkernel for kernel=$kernel1"The error should include "Updated crashkernel=$kdump_crashkernel for kernel=$kernel2"EndSpecify 'kernel1 should have crashkernel updated'When call grubby --info $kernel1The line 3 of output should include crashkernel=$kdump_crashkernelEndSpecify 'kernel2 should have crashkernel updated'When call grubby --info $kernel2The line 3 of output should include crashkernel=$kdump_crashkernelEndEndContext "--kernel=/boot/one-kernel to update one specified kernel"grubby --args crashkernel=$ck --update-kernel ALLSpecify 'kdumpctl should warn the user that crashkernel has been updated'When call reset_crashkernel --kernel=$kernel1The error should include "Updated crashkernel=$kdump_crashkernel for kernel=$kernel1"EndSpecify 'kernel1 should have crashkernel updated'When call grubby --info $kernel1The line 3 of output should include crashkernel=$kdump_crashkernelEndSpecify 'kernel2 should have the old crashkernel'When call grubby --info $kernel2The line 3 of output should include crashkernel=$ckEndEnd- End
- Describe "FADump" fadump
uname() {if [[ $1 == '-m' ]]; thenecho -n ppc64leelif [[ $1 == '-r' ]]; thenecho -n $current_kernelfi}_update_kernel_arg_in_grub_etc_default() {# don't modify /etc/default/grub during the testecho _update_kernel_arg_in_grub_etc_default "$@"}kdump_crashkernel=$(get_default_crashkernel kdump)fadump_crashkernel=$(get_default_crashkernel fadump)Context "when no --kernel specified"grubby --args crashkernel=$ck --update-kernel ALLgrubby --remove-args=fadump --update-kernel ALLSpecify 'kdumpctl should warn the user that crashkernel has been udpated'When call reset_crashkernelThe error should include "Updated crashkernel=$kdump_crashkernel"EndSpecify 'Current running kernel should have crashkernel updated'When call grubby --info $kernel1The line 3 of output should include crashkernel=$kdump_crashkernelEndSpecify 'Other kernel still use the old crashkernel value'When call grubby --info $kernel2The line 3 of output should include crashkernel=$ckEndEndContext "--kernel=ALL --fadump=on"grubby --args crashkernel=$ck --update-kernel ALLSpecify 'kdumpctl should warn the user that crashkernel has been udpated'When call reset_crashkernel --kernel=ALL --fadump=onThe line 1 of output should include "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1"The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel2"EndSpecify 'kernel1 should have crashkernel updated'When call grubby --info $kernel1The line 3 of output should include crashkernel=$fadump_crashkernelEndSpecify 'kernel2 should have crashkernel updated'When call get_grub_kernel_boot_parameter $kernel2 crashkernelThe output should equal $fadump_crashkernelEndEndContext "--kernel=/boot/one-kernel to update one specified kernel"grubby --args crashkernel=$ck --update-kernel ALLgrubby --args fadump=on --update-kernel $kernel1Specify 'kdumpctl should warn the user that crashkernel has been updated'When call reset_crashkernel --kernel=$kernel1The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1"EndSpecify 'kernel1 should have crashkernel updated'When call grubby --info $kernel1The line 3 of output should include crashkernel=$fadump_crashkernelEndSpecify 'kernel2 should have the old crashkernel'When call get_grub_kernel_boot_parameter $kernel2 crashkernelThe output should equal $ckEndEndContext "Update all kernels but without --fadump specified"grubby --args crashkernel=$ck --update-kernel ALLgrubby --args fadump=on --update-kernel $kernel1Specify 'kdumpctl should warn the user that crashkernel has been updated'When call reset_crashkernel --kernel=$kernel1The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1"EndSpecify 'kernel1 should have crashkernel updated'When call get_grub_kernel_boot_parameter $kernel1 crashkernelThe output should equal $fadump_crashkernelEndSpecify 'kernel2 should have the old crashkernel'When call get_grub_kernel_boot_parameter $kernel2 crashkernelThe output should equal $ckEndEndContext 'Switch between fadump=on and fadump=nocma'grubby --args crashkernel=$ck --update-kernel ALLgrubby --args fadump=on --update-kernel ALLSpecify 'fadump=on to fadump=nocma'When call reset_crashkernel --kernel=ALL --fadump=nocmaThe line 1 of output should equal "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"The line 2 of output should equal "_update_kernel_arg_in_grub_etc_default fadump nocma"The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel1"The error should include "Updated crashkernel=$fadump_crashkernel for kernel=$kernel2"EndSpecify 'kernel1 should have fadump=nocma in cmdline'When call get_grub_kernel_boot_parameter $kernel1 fadumpThe output should equal nocmaEndSpecify 'fadump=nocma to fadump=on'When call reset_crashkernel --kernel=ALL --fadump=onThe line 1 of output should equal "_update_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"The line 2 of output should equal "_update_kernel_arg_in_grub_etc_default fadump on"The error should include "Updated fadump=on for kernel=$kernel1"EndSpecify 'kernel2 should have fadump=on in cmdline'When call get_grub_kernel_boot_parameter $kernel1 fadumpThe output should equal onEndEnd- End
+End diff --git a/spec/support/bin/@grubby b/spec/support/bin/@grubby new file mode 100755 index 0000000..2a9b33f --- /dev/null +++ b/spec/support/bin/@grubby @@ -0,0 +1,3 @@ +#!/bin/sh -e +. "$SHELLSPEC_SUPPORT_BIN" +invoke grubby "$@" diff --git a/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf new file mode 100644 index 0000000..b821952 --- /dev/null +++ b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf @@ -0,0 +1,8 @@ +title Fedora (0-rescue-e986846f63134c7295458cf36300ba5b) 33 (Workstation Edition) +version 0-rescue-e986846f63134c7295458cf36300ba5b +linux /boot/vmlinuz-0-rescue-e986846f63134c7295458cf36300ba5b +initrd /boot/initramfs-0-rescue-e986846f63134c7295458cf36300ba5b.img +options root=UUID=45fdf703-3966-401b-b8f7-cf056affd2b0 ro rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 crashkernel=4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-102400T:180G fadump=on +grub_users $grub_users +grub_arg --unrestricted +grub_class kernel diff --git a/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf new file mode 100644 index 0000000..08bd411 --- /dev/null +++ b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf @@ -0,0 +1,8 @@ +title Fedora (5.14.14-200.fc34.x86_64) 34 (Workstation Edition) +version 5.14.14-200.fc34.x86_64 +linux /boot/vmlinuz-5.14.14-200.fc34.x86_64 +initrd /boot/initramfs-5.14.14-200.fc34.x86_64.img +options root=UUID=45fdf703-3966-401b-b8f7-cf056affd2b0 ro rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 crashkernel=4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-102400T:180G fadump=on +grub_users $grub_users +grub_arg --unrestricted +grub_class kernel diff --git a/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf new file mode 100644 index 0000000..9259b99 --- /dev/null +++ b/spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf @@ -0,0 +1,8 @@ +title Fedora (5.15.6-100.fc34.x86_64) 34 (Workstation Edition) +version 5.15.6-100.fc34.x86_64 +linux /boot/vmlinuz-5.15.6-100.fc34.x86_64 +initrd /boot/initramfs-5.15.6-100.fc34.x86_64.img +options root=UUID=45fdf703-3966-401b-b8f7-cf056affd2b0 ro rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 rhgb quiet rd.driver.blacklist=nouveau modprobe.blacklist=nouveau nvidia-drm.modeset=1 crashkernel=4G-16G:768M,16G-64G:1G,64G-128G:2G,128G-1T:4G,1T-2T:6G,2T-4T:12G,4T-8T:20G,8T-16T:36G,16T-32T:64G,32T-64T:128G,64T-102400T:180G fadump=on +grub_users $grub_users +grub_arg --unrestricted +grub_class fedora diff --git a/spec/support/grub_env b/spec/support/grub_env new file mode 100644 index 0000000..a77303c --- /dev/null +++ b/spec/support/grub_env @@ -0,0 +1,3 @@ +# GRUB Environment Block +# WARNING: Do not edit this file by tools other than grub-editenv!!! +################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################## \ No newline at end of file
kdump_get_conf_val allows to retrieves config value defined in kdump.conf and it also support "ext[234]|xfs|btrfs|minix|raw|nfs|ssh".
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdump-lib-initramfs_spec.sh | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 spec/kdump-lib-initramfs_spec.sh
diff --git a/spec/kdump-lib-initramfs_spec.sh b/spec/kdump-lib-initramfs_spec.sh new file mode 100644 index 0000000..4d0a185 --- /dev/null +++ b/spec/kdump-lib-initramfs_spec.sh @@ -0,0 +1,41 @@ +#!/bin/bash +ExampleGroup 'kdump-lib-initramfs' + Include ./kdump-lib-initramfs.sh + + ExampleGroup 'Test kdump_get_conf_val' + KDUMP_CONFIG_FILE=/tmp/kdump_shellspec_test.conf + kdump_config() { + %text + #|default shell + #|nfs my.server.com:/export/tmp # trailing comment + #| failure_action shell + #|dracut_args --omit-drivers "cfg80211 snd" --add-drivers "ext2 ext3" + #|sshkey /root/.ssh/kdump_id_rsa + #|ssh user@my.server.com + } + kdump_config >$KDUMP_CONFIG_FILE + ExampleGroup + # Test the following cases: + # - there is trailing comment + # - there is space before the parameter + # - complicate value for dracut_args + # - Given two parameters, retrive one parameter that has value specified + # - Given two parameters (in reverse order), retrive one parameter that has value specified + Parameters + "#1" nfs my.server.com:/export/tmp + "#2" ssh user@my.server.com + "#3" failure_action shell + "#4" dracut_args '--omit-drivers "cfg80211 snd" --add-drivers "ext2 ext3"' + "#5" 'ssh|aaa' user@my.server.com + "#6" 'aaa|ssh' user@my.server.com + End + + Example 'Get a kernel parameter in different positions' + When call kdump_get_conf_val "$2" + The output should equal "$3" + End + End + + End + +End
Hi Coiby,
On Tue, 1 Mar 2022 14:17:27 +0800 Coiby Xu coxu@redhat.com wrote:
kdump_get_conf_val allows to retrieves config value defined in kdump.conf and it also support "ext[234]|xfs|btrfs|minix|raw|nfs|ssh".
Signed-off-by: Coiby Xu coxu@redhat.com
spec/kdump-lib-initramfs_spec.sh | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 spec/kdump-lib-initramfs_spec.sh
diff --git a/spec/kdump-lib-initramfs_spec.sh b/spec/kdump-lib-initramfs_spec.sh new file mode 100644 index 0000000..4d0a185 --- /dev/null +++ b/spec/kdump-lib-initramfs_spec.sh @@ -0,0 +1,41 @@ +#!/bin/bash +ExampleGroup 'kdump-lib-initramfs'
Is there a reason why you are using ExampleGroup here instead of Context? If not I would prefer to use Context just to be consisted with the other test cases.
Thanks Philipp
- Include ./kdump-lib-initramfs.sh
- ExampleGroup 'Test kdump_get_conf_val'
KDUMP_CONFIG_FILE=/tmp/kdump_shellspec_test.confkdump_config() {%text#|default shell#|nfs my.server.com:/export/tmp # trailing comment#| failure_action shell#|dracut_args --omit-drivers "cfg80211 snd" --add-drivers "ext2 ext3"#|sshkey /root/.ssh/kdump_id_rsa#|ssh user@my.server.com}kdump_config >$KDUMP_CONFIG_FILEExampleGroup# Test the following cases:# - there is trailing comment# - there is space before the parameter# - complicate value for dracut_args# - Given two parameters, retrive one parameter that has value specified# - Given two parameters (in reverse order), retrive one parameter that has value specifiedParameters"#1" nfs my.server.com:/export/tmp"#2" ssh user@my.server.com"#3" failure_action shell"#4" dracut_args '--omit-drivers "cfg80211 snd" --add-drivers "ext2 ext3"'"#5" 'ssh\|aaa' user@my.server.com"#6" 'aaa\|ssh' user@my.server.comEndExample 'Get a kernel parameter in different positions'When call kdump_get_conf_val "$2"The output should equal "$3"EndEnd- End
+End
This test prevents the mistake of adding an option to kdump.conf without changing check_config as is the case with commit 73ced7f ("introduce the auto_reset_crashkernel option to kdump.conf").
Signed-off-by: Coiby Xu coxu@redhat.com --- spec/kdumpctl_general_spec.sh | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index e7d057e..042ef53 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -172,4 +172,12 @@ Describe 'kdumpctl' End End
+ Describe 'check_config()' + It 'should be happy with the default kdump.conf' + KDUMP_CONFIG_FILE=./kdump.conf + When call check_config + The status should be success + End + End + End
Hi Coiby,
On Tue, 1 Mar 2022 14:17:28 +0800 Coiby Xu coxu@redhat.com wrote:
This test prevents the mistake of adding an option to kdump.conf without changing check_config as is the case with commit 73ced7f ("introduce the auto_reset_crashkernel option to kdump.conf").
Signed-off-by: Coiby Xu coxu@redhat.com
spec/kdumpctl_general_spec.sh | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index e7d057e..042ef53 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -172,4 +172,12 @@ Describe 'kdumpctl' End End
- Describe 'check_config()'
It 'should be happy with the default kdump.conf'KDUMP_CONFIG_FILE=./kdump.confWhen call check_config
The function was renamed to parse_config in the meantime.
Thanks Philipp
The status should be successEnd- End
End
Hi Coiby,
I finally had time to look at the series and I'm really impressed! Shellspec looks like it's extremely powerful and the tests will be very useful.
I've got a few comments/questions for some patches. Furthermore I think you should add coverage/ to the .gitignore file. But those are all only small nits. In my opinion you don't need to send a v2 when you address the comments but can simply push the patches.
Great work!
For the series Reviewed-by: Philipp Rudo prudo@redhat.com
On Tue, 1 Mar 2022 14:17:20 +0800 Coiby Xu coxu@redhat.com wrote:
v1:
- issues found by Tao
- clean up files creatd by mktemp
- use DUMMY_PARAM when calling a mocked
- clean up unsued parameters
- mock uname for kdump mode tests to fake current running kernel
- {add,remove}_kernel_arg_in_grub_etc_default replaced by _update_*
Potential benefits brought by unit tests
- catch as many bugs as possible and catch them as early as possible
- improve the code quality and make the code robust
- Keep API stable
- prevent regressions from change during the development and in the future (it's quite annoying to solve one problem only to find another cropping up)
- make it easy for code review because of concrete test cases
Why ShellSpec
ShellSpec [1] is chosen for the following benefits,
- rich features: to name a few
- mocking
- Parameterized tests
- code coverage
- good documentation and examples
For the full comparison between shellspec and other frameworks, please refer to [2].
How to run the tests
After installing shellspec and optionally kcov $ curl -fsSL https://git.io/shellspec | sh $ sudo dnf install kcov
go to the root of this repo
create a file with _spec as suffix in the spec folder, e.g. spec/kdumpctl_spec.sh
run shellspec shellspec --kcov --kcov-options "--include-pattern=kdumpctl,kdump-lib.sh,kdump-lib-initramfs.sh,kdump-logger.sh"
Running: /bin/sh [bash 5.1.0(1)-release] ................................................................
Finished in 11.15 seconds (user 6.97 seconds, sys 9.54 seconds) 64 examples, 0 failures
Code covered: 16.08%, Executed lines: 224, Instrumented lines: 1393
[1] https://github.com/shellspec/shellspec [2] https://github.com/dodie/testing-in-bash#detailed-comparision
Coiby Xu (8): unit tests: prepare for kdumpctl and kdump-lib.sh to be unit-tested unit tests: add tests for get_grub_kernel_boot_parameter unit tests: add tests for get_dump_mode_by_fadump_val unit tests: add tests for kdumpctl read_proc_environ_var and _is_osbuild unit tests: add tests for _{update,read}_kernel_arg_in_grub_etc_default in kdumpctl unit tests: add tests for "kdumpctl reset-crashkernel" unit tests: add tests for kdump_get_conf_val in kdump-lib-initramfs.sh unit tests: add check_config with with the default kdump.conf
.shellspec | 0 kdump-lib.sh | 7 +- kdumpctl | 24 +- spec/kdump-lib-initramfs_spec.sh | 41 ++++ spec/kdumpctl_general_spec.sh | 183 ++++++++++++++ spec/kdumpctl_reset_crashkernel_spec.sh | 223 ++++++++++++++++++ spec/support/bin/@grubby | 3 + ...846f63134c7295458cf36300ba5b-0-rescue.conf | 8 + ...58cf36300ba5b-5.14.14-200.fc34.x86_64.conf | 8 + ...458cf36300ba5b-5.15.6-100.fc34.x86_64.conf | 8 + spec/support/grub_env | 3 + 11 files changed, 499 insertions(+), 9 deletions(-) create mode 100644 .shellspec create mode 100644 spec/kdump-lib-initramfs_spec.sh create mode 100644 spec/kdumpctl_general_spec.sh create mode 100644 spec/kdumpctl_reset_crashkernel_spec.sh create mode 100755 spec/support/bin/@grubby create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf create mode 100644 spec/support/grub_env
Hi Philipp,
On Fri, Apr 08, 2022 at 03:54:18PM +0200, Philipp Rudo wrote:
Hi Coiby,
I finally had time to look at the series and I'm really impressed! Shellspec looks like it's extremely powerful and the tests will be very useful.
Glad to know these tests will be useful:)
I've got a few comments/questions for some patches. Furthermore I think you should add coverage/ to the .gitignore file. But those are all only small nits. In my opinion you don't need to send a v2 when you address the comments but can simply push the patches.
All your suggestions have been applied! I've pushed the patches to the rawhide branch with additional changes, - add a counter case for parse_check i.e. parse_check should complain about unknown option like blabla. - since the ShellSpec's DSL is compatible with shell scripts, I fix some shellcheck warnings with "shellcheck -f diff spec/* | git apply"
Great work!
For the series Reviewed-by: Philipp Rudo prudo@redhat.com
Thanks to you and Tao as well for reviewing the patches!
On Tue, 1 Mar 2022 14:17:20 +0800 Coiby Xu coxu@redhat.com wrote:
v1:
- issues found by Tao
- clean up files creatd by mktemp
- use DUMMY_PARAM when calling a mocked
- clean up unsued parameters
- mock uname for kdump mode tests to fake current running kernel
- {add,remove}_kernel_arg_in_grub_etc_default replaced by _update_*
Potential benefits brought by unit tests
- catch as many bugs as possible and catch them as early as possible
- improve the code quality and make the code robust
- Keep API stable
- prevent regressions from change during the development and in the future (it's quite annoying to solve one problem only to find another cropping up)
- make it easy for code review because of concrete test cases
Why ShellSpec
ShellSpec [1] is chosen for the following benefits,
- rich features: to name a few
- mocking
- Parameterized tests
- code coverage
- good documentation and examples
For the full comparison between shellspec and other frameworks, please refer to [2].
How to run the tests
After installing shellspec and optionally kcov $ curl -fsSL https://git.io/shellspec | sh $ sudo dnf install kcov
go to the root of this repo
create a file with _spec as suffix in the spec folder, e.g. spec/kdumpctl_spec.sh
run shellspec shellspec --kcov --kcov-options "--include-pattern=kdumpctl,kdump-lib.sh,kdump-lib-initramfs.sh,kdump-logger.sh"
Running: /bin/sh [bash 5.1.0(1)-release] ................................................................
Finished in 11.15 seconds (user 6.97 seconds, sys 9.54 seconds) 64 examples, 0 failures
Code covered: 16.08%, Executed lines: 224, Instrumented lines: 1393
[1] https://github.com/shellspec/shellspec [2] https://github.com/dodie/testing-in-bash#detailed-comparision
Coiby Xu (8): unit tests: prepare for kdumpctl and kdump-lib.sh to be unit-tested unit tests: add tests for get_grub_kernel_boot_parameter unit tests: add tests for get_dump_mode_by_fadump_val unit tests: add tests for kdumpctl read_proc_environ_var and _is_osbuild unit tests: add tests for _{update,read}_kernel_arg_in_grub_etc_default in kdumpctl unit tests: add tests for "kdumpctl reset-crashkernel" unit tests: add tests for kdump_get_conf_val in kdump-lib-initramfs.sh unit tests: add check_config with with the default kdump.conf
.shellspec | 0 kdump-lib.sh | 7 +- kdumpctl | 24 +- spec/kdump-lib-initramfs_spec.sh | 41 ++++ spec/kdumpctl_general_spec.sh | 183 ++++++++++++++ spec/kdumpctl_reset_crashkernel_spec.sh | 223 ++++++++++++++++++ spec/support/bin/@grubby | 3 + ...846f63134c7295458cf36300ba5b-0-rescue.conf | 8 + ...58cf36300ba5b-5.14.14-200.fc34.x86_64.conf | 8 + ...458cf36300ba5b-5.15.6-100.fc34.x86_64.conf | 8 + spec/support/grub_env | 3 + 11 files changed, 499 insertions(+), 9 deletions(-) create mode 100644 .shellspec create mode 100644 spec/kdump-lib-initramfs_spec.sh create mode 100644 spec/kdumpctl_general_spec.sh create mode 100644 spec/kdumpctl_reset_crashkernel_spec.sh create mode 100755 spec/support/bin/@grubby create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-0-rescue.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.14.14-200.fc34.x86_64.conf create mode 100644 spec/support/boot_load_entries/e986846f63134c7295458cf36300ba5b-5.15.6-100.fc34.x86_64.conf create mode 100644 spec/support/grub_env
-
Coiby Xu -
Philipp Rudo