Re: [ndctl PATCH v3] test/monitor.sh: replace sleep with event driven wait

[Marc Herbert <marc.herbert@linux.intel.com>]

On 2025-05-19 12:28, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> 
> Again, looking for Tested-by Tags. Thanks!

Wow, it went so fast I genuinely thought something went terribly wrong
and it did not test anything anymore. While reporting "Success!" - that
has happened before.

But I check the test logs and it does a lot of stuff - in less 2
seconds! What a difference.

Tested-by: Marc Herbert <marc.herbert@linux.intel.com>

I have only concern holding back my Reviewed-by + some minor nits after
the code. I think the error message for the timeout pipeline is too
limited, terse and generic; does not say anything about what happened,
does not make the difference between a timeout and some other, unexpected
failure, how many occurences were found etc. I think the error clause
should do at least two things:

- print the timeout $? exit code.
- grep the log file again, either with -c if it's too long, or not.
- Any other useful information that could be up for grabs at that point.


> +wait_for_logfile_update()
> +{
> +	local expect_string="$1"
> +	local expect_count="$2"
> +
> +	# Wait up to 3s for $expect_count occurrences of $expect_string
> +	# tail -n +1 -F: starts watching the logfile from the first line
> +
> +	if ! timeout 3s tail -n +1 -F "$logfile" | grep -m "$expect_count" -q "$expect_string"; then
> +		echo "logfile not updated in 3 secs"
> +		err "$LINENO"
> +	fi
> +}

tail -F really does solve the problem with very little code, well
spotted!
Nit 1: I did not know that -F option: can you try to sneak the word
"retry" somewhere in the comments?

Codestyle Nit 2: to avoid negations I generally prefer:

  timeout 3s tail -n +1 -F "$logfile" |
     grep -q -m "$expect_count" -q "$expect_string" || {
      error ...
    }

This also translates to plain English nicely as "wait or fail", "do or
die", etc.

Super minor nit 3: "$expect_string" looks like an argument of "-q", can
you move -q first?