[RFC 0/8] unit tests with ShellSpec

[Coiby Xu]

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
    _{add,remove,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                 | 175 ++++++++++++++
 spec/kdumpctl_reset_crashkernel_spec.sh       | 216 ++++++++++++++++++
 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, 484 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

[RFC 1/8] unit tests: prepare for kdumpctl and kdump-lib.sh to be unit-tested

[Coiby Xu]

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 
---
 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 271c60c..aa3ad69 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
@@ -1727,11 +1733,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
@@ -1804,6 +1805,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

[RFC 2/8] unit tests: add tests for get_grub_kernel_boot_parameter

[Coiby Xu]

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 
---
 .shellspec                    |  0
 spec/kdumpctl_general_spec.sh | 46 +++++++++++++++++++++++++++++++++++
 2 files changed, 46 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..8c7ec01
--- /dev/null
+++ b/spec/kdumpctl_general_spec.sh
@@ -0,0 +1,46 @@
+#!/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"
+		}
+		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 /boot/vmlinuz "$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 /boot/vmlinuz 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

[RFC 2/8] unit tests: add tests for get_grub_kernel_boot_parameter

[Tao Liu]

On Mon, Feb 21, 2022 at 8:58 PM Coiby Xu  wrote:
> 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
> 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
>
> .shellspec                    |  0
>  spec/kdumpctl_general_spec.sh | 46 +++++++++++++++++++++++++++++++++++
>  2 files changed, 46 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..8c7ec01
> --- /dev/null
> +++ b/spec/kdumpctl_general_spec.sh
> @@ -0,0 +1,46 @@
> +#!/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"
>           }
>           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 /boot/vmlinuz "$2"

Can we change the /boot/vmlinuz string into variable and use it like:
    DUMMY_PARAM=/boot/vmlinuz
    When call get_grub_kernel_boot_parameter $DUMMY_PARAM "$2"

"/boot/vmlinuz" is a little misleading to me, because I didn't have
such a file on my local
drive. As far as I understand the code, the previous grubby() function
just mock up the
grubby cmdline tool. So any string other than "ALL" as the 1st param given to
get_grub_kernel_boot_parameter, the grubby will always output the mocked text,
so "DUMMY_PARAM" is more likely to represent the idea.
>                           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 /boot/vmlinuz crashkernel

Same issue here.
>                   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
>
> 2.34.1
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 2/8] unit tests: add tests for get_grub_kernel_boot_parameter

[Coiby Xu]

On Mon, Feb 28, 2022 at 03:41:09PM +0800, Tao Liu wrote:
> On Mon, Feb 21, 2022 at 8:58 PM Coiby Xu  wrote:
> > 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
> > 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
> >
> > .shellspec                    |  0
> >  spec/kdumpctl_general_spec.sh | 46 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 46 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..8c7ec01
> > --- /dev/null
> > +++ b/spec/kdumpctl_general_spec.sh
> > @@ -0,0 +1,46 @@
> > +#!/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"
> >           }
> >           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 /boot/vmlinuz "$2"
>
> Can we change the /boot/vmlinuz string into variable and use it like:
>    DUMMY_PARAM=/boot/vmlinuz
>    When call get_grub_kernel_boot_parameter $DUMMY_PARAM "$2"
>
> "/boot/vmlinuz" is a little misleading to me, because I didn't have
> such a file on my local
> drive. As far as I understand the code, the previous grubby() function
> just mock up the
> grubby cmdline tool. So any string other than "ALL" as the 1st param given to
> get_grub_kernel_boot_parameter, the grubby will always output the mocked text,
> so "DUMMY_PARAM" is more likely to represent the idea.
> >                           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 /boot/vmlinuz crashkernel
>
> Same issue here.

Applied to v1. Thanks for the suggestion!
> >                   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
> >
> > 2.34.1
> > _______________________________________________
> > kexec mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 3/8] unit tests: add tests for get_dump_mode_by_fadump_val

[Coiby Xu]

Signed-off-by: Coiby Xu 
---
 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 8c7ec01..2f60daa 100644
--- a/spec/kdumpctl_general_spec.sh
+++ b/spec/kdumpctl_general_spec.sh
@@ -43,4 +43,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

[RFC 4/8] unit tests: add tests for kdumpctl read_proc_environ_var and _is_osbuild

[Coiby Xu]

Signed-off-by: Coiby Xu 
---
 spec/kdumpctl_general_spec.sh | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
index 2f60daa..86894d8 100644
--- a/spec/kdumpctl_general_spec.sh
+++ b/spec/kdumpctl_general_spec.sh
@@ -68,5 +68,35 @@ Describe 'kdumpctl'

End

+	Describe "read_proc_environ_var()"
+		environ_test_file=$(mktemp -t spec_test_environ_test_file.XXXXXXXXXX)
+		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

[RFC 4/8] unit tests: add tests for kdumpctl read_proc_environ_var and _is_osbuild

[Tao Liu]

On Mon, Feb 21, 2022 at 08:57:42PM +0800, Coiby Xu wrote:
> Signed-off-by: Coiby Xu
>
> spec/kdumpctl_general_spec.sh | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
> index 2f60daa..86894d8 100644
> --- a/spec/kdumpctl_general_spec.sh
> +++ b/spec/kdumpctl_general_spec.sh
> @@ -68,5 +68,35 @@ Describe 'kdumpctl'
>
> End
> Describe "read_proc_environ_var()"
> environ_test_file=$(mktemp -t spec_test_environ_test_file.XXXXXXXXXX)
> 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

Should we add a clean up here?
    	rm -f $environ_test_file
> 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

I noticed there are a few places where the temp files are not cleaned up
after mktemp. These files should be cleaned, otherwise /tmp folder will
be messed up.
> End
> End
>
> 2.34.1
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 4/8] unit tests: add tests for kdumpctl read_proc_environ_var and _is_osbuild

[Coiby Xu]

On Mon, Feb 28, 2022 at 03:55:57PM +0800, Tao Liu wrote:
> On Mon, Feb 21, 2022 at 08:57:42PM +0800, Coiby Xu wrote:
> > Signed-off-by: Coiby Xu
> >
> > spec/kdumpctl_general_spec.sh | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
> > index 2f60daa..86894d8 100644
> > --- a/spec/kdumpctl_general_spec.sh
> > +++ b/spec/kdumpctl_general_spec.sh
> > @@ -68,5 +68,35 @@ Describe 'kdumpctl'
> >
> > End
> > Describe "read_proc_environ_var()"
> > environ_test_file=$(mktemp -t spec_test_environ_test_file.XXXXXXXXXX)
> > 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
>
> Should we add a clean up here?
>    	rm -f $environ_test_file
> > 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
>
> I noticed there are a few places where the temp files are not cleaned up
> after mktemp. These files should be cleaned, otherwise /tmp folder will
> be messed up.

Thanks for catching this kind of issue! v1 has cleaned up these files.
> > End
> > End
> >
> > 2.34.1
> > _______________________________________________
> > kexec mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 5/8] unit tests: add tests for _{add,remove,read}_kernel_arg_in_grub_etc_default in kdumpctl

[Coiby Xu]

Signed-off-by: Coiby Xu 
---
 spec/kdumpctl_general_spec.sh | 65 +++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
index 86894d8..a18aac0 100644
--- a/spec/kdumpctl_general_spec.sh
+++ b/spec/kdumpctl_general_spec.sh
@@ -99,4 +99,69 @@ Describe 'kdumpctl'
    	End
    End

+	Describe '_add_kernel_arg_in_grub_etc_default()'
+		GRUB_ETC_DEFAULT=/tmp/default_grub
+
+		Context 'when the given parameter is in different positions'
+		 Parameters
+		 	"crashkernel=222M fadump=on rhgb quiet" crashkernel 333M 222M
+		 	" fadump=on crashkernel=222M rhgb quiet" crashkernel 333M 222M
+		 	"fadump=on rhgb quiet crashkernel=222M" crashkernel 333M 222M
+		 	"fadump=on rhgb quiet" crashkernel 333M
+		 	"fadump=on foo=bar1 rhgb quiet" foo bar2 bar1
+		 End
+
+		 It 'should update the kernel parameter correctly'
+		 	echo 'GRUB_CMDLINE_LINUX="'"$1"'"' >$GRUB_ETC_DEFAULT
+		 	When call _add_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 _add_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 _add_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
+
+	End
+
+
+	Describe '_remove_kernel_arg_in_grub_etc_default()'
+		GRUB_ETC_DEFAULT=/tmp/default_grub
+		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 _remove_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 _remove_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
+
+
+	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

[RFC 5/8] unit tests: add tests for _{add,remove,read}_kernel_arg_in_grub_etc_default in kdumpctl

[Tao Liu]

On Mon, Feb 21, 2022 at 08:57:43PM +0800, Coiby Xu wrote:
> Signed-off-by: Coiby Xu
>
> spec/kdumpctl_general_spec.sh | 65 +++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
> index 86894d8..a18aac0 100644
> --- a/spec/kdumpctl_general_spec.sh
> +++ b/spec/kdumpctl_general_spec.sh
> @@ -99,4 +99,69 @@ Describe 'kdumpctl'
>    	End
>    End
> Describe '_add_kernel_arg_in_grub_etc_default()'
> GRUB_ETC_DEFAULT=/tmp/default_grub
> Context 'when the given parameter is in different positions'
>  Parameters
>  	"crashkernel=222M fadump=on rhgb quiet" crashkernel 333M 222M

^^^^
>  	" fadump=on crashkernel=222M rhgb quiet" crashkernel 333M 222M

^^^^
>  	"fadump=on rhgb quiet crashkernel=222M" crashkernel 333M 222M

^^^^
>  	"fadump=on rhgb quiet" crashkernel 333M
>  	"fadump=on foo=bar1 rhgb quiet" foo bar2 bar1

^^^^
>  End

Shouldn't these parameters be removed? I see from the following test
case, only "$2=$3" is used, so the "$4" parameters should be removed
for avoid misleading.
>  It 'should update the kernel parameter correctly'
>  	echo 'GRUB_CMDLINE_LINUX="'"$1"'"' >$GRUB_ETC_DEFAULT
>  	When call _add_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 _add_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 _add_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
> End
> Describe '_remove_kernel_arg_in_grub_etc_default()'
> GRUB_ETC_DEFAULT=/tmp/default_grub
> 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 _remove_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 _remove_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
> 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

For the above $GRUB_ETC_DEFAULT should be cleaned after test.
> End
>
> 2.34.1
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 5/8] unit tests: add tests for _{add,remove,read}_kernel_arg_in_grub_etc_default in kdumpctl

[Coiby Xu]

On Mon, Feb 28, 2022 at 04:31:33PM +0800, Tao Liu wrote:
> On Mon, Feb 21, 2022 at 08:57:43PM +0800, Coiby Xu wrote:
> > Signed-off-by: Coiby Xu
> >
> > spec/kdumpctl_general_spec.sh | 65 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >
> > diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh
> > index 86894d8..a18aac0 100644
> > --- a/spec/kdumpctl_general_spec.sh
> > +++ b/spec/kdumpctl_general_spec.sh
> > @@ -99,4 +99,69 @@ Describe 'kdumpctl'
> >    	End
> >    End
> > Describe '_add_kernel_arg_in_grub_etc_default()'
> > GRUB_ETC_DEFAULT=/tmp/default_grub
> > Context 'when the given parameter is in different positions'
> >  Parameters
> >  	"crashkernel=222M fadump=on rhgb quiet" crashkernel 333M 222M
>                                                                             ^^^^
> >  	" fadump=on crashkernel=222M rhgb quiet" crashkernel 333M 222M
>                                                                              ^^^^
> >  	"fadump=on rhgb quiet crashkernel=222M" crashkernel 333M 222M
>                                                                             ^^^^
> >  	"fadump=on rhgb quiet" crashkernel 333M
> >  	"fadump=on foo=bar1 rhgb quiet" foo bar2 bar1
>                                                             ^^^^
> >  End
>
> Shouldn't these parameters be removed? I see from the following test
> case, only "$2=$3" is used, so the "$4" parameters should be removed
> for avoid misleading.

Good catch! They are leftover of an early version. Removed in v1.
Thanks!
> >  It 'should update the kernel parameter correctly'
> >  	echo 'GRUB_CMDLINE_LINUX="'"$1"'"' >$GRUB_ETC_DEFAULT
> >  	When call _add_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 _add_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 _add_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
> > End
> > Describe '_remove_kernel_arg_in_grub_etc_default()'
> > GRUB_ETC_DEFAULT=/tmp/default_grub
> > 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 _remove_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 _remove_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
> > 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
>
> For the above $GRUB_ETC_DEFAULT should be cleaned after test.
> > End
> >
> > 2.34.1
> > _______________________________________________
> > kexec mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 6/8] unit tests: add tests for "kdumpctl reset-crashkernel"

[Coiby Xu]

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 
---
 spec/kdumpctl_reset_crashkernel_spec.sh       | 216 ++++++++++++++++++
 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, 246 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..de6d40a
--- /dev/null
+++ b/spec/kdumpctl_reset_crashkernel_spec.sh
@@ -0,0 +1,216 @@
+#!/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)
+
+	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)
+		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
+		current_kernel=5.15.6-100.fc34.x86_64
+		uname() {
+			if [[ $1 == '-m' ]]; then
+				echo -n ppc64le
+			elif [[ $1 == '-r' ]]; then
+				echo -n $current_kernel
+			fi
+		}
+
+		_add_kernel_arg_in_grub_etc_default() {
+			# don't modify /etc/default/grub during the test
+			echo _add_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 "_add_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 "_add_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"
+				The line 2 of output should equal "_add_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 "_add_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"
+				The line 2 of output should equal "_add_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

[RFC 6/8] unit tests: add tests for "kdumpctl reset-crashkernel"

[Tao Liu]

On Mon, Feb 21, 2022 at 08:57:44PM +0800, Coiby Xu 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
>
> spec/kdumpctl_reset_crashkernel_spec.sh       | 216 ++++++++++++++++++
>  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, 246 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..de6d40a
> --- /dev/null
> +++ b/spec/kdumpctl_reset_crashkernel_spec.sh
> @@ -0,0 +1,216 @@
> +#!/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)
> 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)
> 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

There are a few issues:

1) I encountered the following error for this test:
     1.1) Example aborted (exit status: 1)
            kdump: kernel 5.14.17-301.fc35.x86_64 doesn't exist
            kdump: no running kernel found
The issue is when calling reset_crashkernel, it will call
_find_kernel_path_by_release, then call _grubby_kernel_str=$(grubby
--info ALL | ..., grubby will encounter the permission issue
when run as non-root:

$ grubby --info ALL
    grep: /boot/grub2/grubenv: Permission denied

Should it be mocked, instead of actually reading the local
drive for the file?

2) In 1st test, after reset_crashkernel is called, it actually checks the
following:

expected "The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
            kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-5.14.17-301.fc35.x86_64+debug. Please reboot the system for the change to take effect.
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
            kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-5.14.17-301.fc35.x86_64. Please reboot the system for the change to take effect." to include "Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M"

The /boot/vmlinuz-5.14.17-301* is the kernel of my local drive. The 1st
test will pass, however as the string indicates, will the test actually
make changes to local grub configuration?

3) I didn't quite understand the test.

"grubby --args crashkernel=$ck --update-kernel ALL" will update boot_load_entries
files with crashkernel=222M, then reset_crashkernel() is called, do you
want to update boot_load_entries files with original crashkernel value?

Then "grubby --info $kernel1" and then "grubby --info $kernel2", according
to the description, kernel1 is "running kernel", kernel2 is "other kernel",
is it the case of your machine status? Because I only have
5.14.17-301.fc35.x86_64 kernel on my machine, so kernel1 and kernel2 are
all "other kernel" to me, thus the test fails for me...
> 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
> current_kernel=5.15.6-100.fc34.x86_64
> uname() {
> 	if [[ $1 == '-m' ]]; then
> 		echo -n ppc64le
> 	elif [[ $1 == '-r' ]]; then
> 		echo -n $current_kernel
> 	fi
> }
> _add_kernel_arg_in_grub_etc_default() {
> 	# don't modify /etc/default/grub during the test
> 	echo _add_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 "_add_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 "_add_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"
> 		The line 2 of output should equal "_add_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 "_add_kernel_arg_in_grub_etc_default crashkernel $fadump_crashkernel"
> 		The line 2 of output should equal "_add_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
> -- 
> 2.34.1
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[RFC 6/8] unit tests: add tests for "kdumpctl reset-crashkernel"

[Coiby Xu]

On Mon, Feb 28, 2022 at 05:26:02PM +0800, Tao Liu wrote:
> On Mon, Feb 21, 2022 at 08:57:44PM +0800, Coiby Xu 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
> >
> > spec/kdumpctl_reset_crashkernel_spec.sh       | 216 ++++++++++++++++++
> >  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, 246 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..de6d40a
> > --- /dev/null
> > +++ b/spec/kdumpctl_reset_crashkernel_spec.sh
> > @@ -0,0 +1,216 @@
> > +#!/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)
> > 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)
> > 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
>
> There are a few issues:
> I encountered the following error for this test:
>  1.1) Example aborted (exit status: 1)
>         kdump: kernel 5.14.17-301.fc35.x86_64 doesn't exist
>         kdump: no running kernel found
>
> The issue is when calling reset_crashkernel, it will call
> _find_kernel_path_by_release, then call _grubby_kernel_str=$(grubby
> --info ALL | ..., grubby will encounter the permission issue
> when run as non-root:
>
> $ grubby --info ALL
>    grep: /boot/grub2/grubenv: Permission denied
>
> Should it be mocked, instead of actually reading the local
> drive for the file?
> In 1st test, after reset_crashkernel is called, it actually checks the
>
> following:
>
> expected "The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64+debug is incorrect
>            kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-5.14.17-301.fc35.x86_64+debug. Please reboot the system for the change to take effect.
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
>            The param /boot/vmlinuz-5.14.17-301.fc35.x86_64 is incorrect
>            kdump: Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M for kernel=/boot/vmlinuz-5.14.17-301.fc35.x86_64. Please reboot the system for the change to take effect." to include "Updated crashkernel=1G-4G:192M,4G-64G:256M,64G-:512M"
>
> The /boot/vmlinuz-5.14.17-301* is the kernel of my local drive. The 1st
> test will pass, however as the string indicates, will the test actually
> make changes to local grub configuration?
> I didn't quite understand the test.
>
> "grubby --args crashkernel=$ck --update-kernel ALL" will update boot_load_entries
> files with crashkernel=222M, then reset_crashkernel() is called, do you
> want to update boot_load_entries files with original crashkernel value?

"grubby --args crashkernel=$ck --update-kernel ALL" is used to reset
the crashkernel values of all kernels to a initial state i.e. crashkernel=$ck.
> Then "grubby --info $kernel1" and then "grubby --info $kernel2", according
> to the description, kernel1 is "running kernel", kernel2 is "other kernel",
> is it the case of your machine status? Because I only have
> 5.14.17-301.fc35.x86_64 kernel on my machine, so kernel1 and kernel2 are
> all "other kernel" to me, thus the test fails for me...

Thanks for running the unit-tests to catch these issues! The root cause of
these issues is I haven't faked the current running kernel. In v1, I've
mocked `uname -r` so the current running kernel is faked.

[RFC 7/8] unit tests: add tests for kdump_get_conf_val in kdump-lib-initramfs.sh

[Coiby Xu]

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 
---
 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 [e-mail]
+		}
+		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 [e-mail]
+				"#3" failure_action shell
+				"#4" dracut_args '--omit-drivers "cfg80211 snd" --add-drivers "ext2 ext3"'
+				"#5" 'ssh|aaa' [e-mail]
+				"#6" 'aaa|ssh' [e-mail]
+			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

[RFC 8/8] unit tests: add check_config with with the default kdump.conf

[Coiby Xu]

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 
---
 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 a18aac0..83f1d49 100644
--- a/spec/kdumpctl_general_spec.sh
+++ b/spec/kdumpctl_general_spec.sh
@@ -164,4 +164,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

[Tao Liu]

Hi Coiby,

The test suite is great!

On Mon, Feb 21, 2022 at 08:57:38PM +0800, Coiby Xu wrote:
> 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

However, by trying the test on my local machine, I noticed there are a few
failing cases, maybe due to our env differences. So I'm thinking about 
running the test in a docker container, which will prevent the
shellspec installing, file modification and temporary files creating
on the local drive.

Do you have any plan/preferences for shellspec integration with kexec-tools?

For example:
1) docker container
2) virtual machine disk image
3) physical host, with careful mock and temp-file cleanup.
4) rpm dependency
5) ???

Thanks,
Tao Liu
> [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
>     _{add,remove,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                 | 175 ++++++++++++++
>  spec/kdumpctl_reset_crashkernel_spec.sh       | 216 ++++++++++++++++++
>  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, 484 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
>
> -- 
> 2.34.1
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure

[Coiby Xu]

Hi Tao,

On Mon, Feb 28, 2022 at 06:44:06PM +0800, Tao Liu wrote:
> Hi Coiby,
>
> The test suite is great!

Thanks for reviewing the patches!
> On Mon, Feb 21, 2022 at 08:57:38PM +0800, Coiby Xu wrote:
> > 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
>
> However, by trying the test on my local machine, I noticed there are a few
> failing cases, maybe due to our env differences. So I'm thinking about
> running the test in a docker container, which will prevent the
> shellspec installing, file modification and temporary files creating
> on the local drive.

Sorry for these issues! v1 should be run smoothly after installing
shellspec.
> Do you have any plan/preferences for shellspec integration with kexec-tools?
>
> For example:
> docker container
> virtual machine disk image
> physical host, with careful mock and temp-file cleanup.
> rpm dependency
> ???

I have an idea to change our workflow from email-based workflow to
gitlab/github-based workflow. So the unit-tests and CI tests (i.e. tests in
the tests folder) can be triggered automatically when an pull/merge
request is created or updated. To achieve this goal, we have to be able
to run tests inside a docker container (at least this required by
github. Gitlab may also requires this).
> Thanks,
> Tao Liu
> > [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
> >     _{add,remove,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                 | 175 ++++++++++++++
> >  spec/kdumpctl_reset_crashkernel_spec.sh       | 216 ++++++++++++++++++
> >  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, 484 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
> >
> > --
> > 2.34.1
> > _______________________________________________
> > kexec mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure