Hi Coiby,
On Wed, 17 May 2023 18:20:34 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
[...]
>> + Describe "When kexec-tools have its default
crashkernel updated, "
>> +
>> + BeforeAll 'setup'
>> + AfterAll 'cleanup'
>> +
>> + Context "if kexec-tools is updated alone, "
>> + Specify 'reset_crashkernel_after_update should report updated kernels
and note that auto_reset_crashkernel=yes'
>> + When call reset_crashkernel_after_update
>> + The output should include "For kernel=$kernel1,
crashkernel=$kdump_crashkernel now."
>> + The output should not include "For kernel=$kernel2,
crashkernel=$kdump_crashkernel now."
>> + The output should include "auto_reset_crashkernel=no"
>
>I find the last check quite confusing. First I thought that you return
>the value auto_reset_crashkernel is set to. But it is only a hint on
>how to turn off the auto update. Personally I would simply remove the
>line.
Sorry it confuses you. Personally, I think we should let users know this
hint otherwise users may complain somehow there custom crasherkernel
doesn't persist. How about adding a comment to explain it?
No problem. I agree that having the note makes sense. But personally I
don't think it's important enough to add a test case for it. Anyway, if
you add a short comment I'm fine with keeping it.
BTW, I've noticed that you are simply echo'ing the line you are
checking for here. Is that intended? Wouldn't dinfo make more sense, so
users can silence the message if they desire.
Thanks
Philipp