Re: [ndctl PATCH v3] test: fail on unexpected kernel error & warning, not just "Call Trace"

[Dave Jiang <dave.jiang@intel.com>]

On 6/11/25 4:44 PM, marc.herbert@linux.intel.com wrote:
> v3 changes:
> 
> - One-line fix of kmsg_no_fail_on exclusion of the warning "Extended
>   linear cache calculation failed". Fixes test failures since kernel
>   commit de7fb30a5870 ("Add extended linear cache support for CXL").
> 
> v2 major changes:
> 
> - The old $SECONDS variable is dropped from journalctl. Which allows:
> - ... dropping the very short-lived COOLDOWN proposed in version 1.
> - A new, optional NDTEST_LOG_DBG code which allows "stress testing"
>   the approach and proving that it is safe.
> 
> I tested and compared for many hours $SECONDS versus the NDTEST_START
> approach that Alison submitted a few months ago and the conclusion is
> very clear:
> - $SECONDS does require a ~1.2 cool down between every test. As it was
>   done in v1.
> - NDTEST_START requires zero cool down.
> 
> So that is why I dropped $SECONDS and the cool down.
> 
> 
>> Split them into a patchset for easier review and then I'll take a
>> look. Thanks!
> 
> There are 3 logical changes in the main commit:
> 
> A1) Dropping $SECONDS, replaced with NDTEST_START
> 
> A2) The new NDTEST_LOG_DBG which was/is critical for:
>    - proving that $SECONDS required a "cool down" (with version 1)
>    - proving NDTEST_START does _not_ require any cool down, safe
>      even without any.
> 
> B) The new, _harden_ journalctl check in check_dmesg() and its
>    kmsg_fail_if_missing and kmsg_no_fail_on. The main feature!
> 
> 
> - B) requires A1) because $SECONDS is too imprecise. With B) only, the
>   tests fail.
> 
> - The A2) test code achieves nothing without B), it cannot prove
>   anything without B).
> 
> - A1) and A2) are logically independent but their code are fairly
>   intertwined and very painful to separate. Plus, B) would have to sit
>   in the middle: A1->B->A2
> 
> Long story short:
> 
> - while they could be logically separate, these changes are tightly
> coupled with each other.
> 
> - breaking down that (relatively small) commit is theoretically
> possible but would be very labor intensive. I know because I just went
> through a similar "git action"  to compare $SECONDS versus
> NDTEST_START for COOLDOWN reasons and it was not fun at all.
> 
> 


Not bash expert, but LGTM for the series
Reviewed-by: Dave Jiang <dave.jiang@intel.com>