5:23 a.m.
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(a)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(a)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
>
> 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
>
--
Best regards,
Coiby