alison.schofield@ wrote: > From: Alison Schofield <alison.schofi...@intel.com> > > monitor.sh runs for 50 seconds and spends 48 of those seconds sleeping > after sync. It sleeps for 3 seconds each time it restarts the monitor, > and 3 seconds before checking for expected log entries, a total of > 16 naps. > > Replace the sleeps with polling that waits for a max of 3 seconds but > does so in 30 0.1 second intervals. This leaves the current behavior > in place but offers a shorter run time for system configs capable of > a faster sync. > > Again - I'd like to see some Tested-by's on this one because it wouldn't > be the first time my golden environment wasn't representative of all > environments where these tests are run. Thanks! > > Signed-off-by: Alison Schofield <alison.schofi...@intel.com> > --- > > Changes in v2: > - Poll for 3 seconds instead of removing sleep entirely (MarcH) > - Update commit msg & log > Link to v1: > https://lore.kernel.org/nvdimm/20250514014133.1431846-1-alison.schofi...@intel.com/ > > test/monitor.sh | 43 ++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/test/monitor.sh b/test/monitor.sh > index be8e24d6f3aa..61cad098d87c 100755 > --- a/test/monitor.sh > +++ b/test/monitor.sh > @@ -21,12 +21,45 @@ trap 'err $LINENO' ERR > > check_min_kver "4.15" || do_skip "kernel $KVER may not support monitor > service" > > +wait_for_logfile_update() > +{ > + local file="$1" > + local prev_size="$2" > + local timeout=30 > + local i=0 > + > + # prev_size is always zero because start_monitor truncates it. > + # Set and check against it anyway to future proof. > + while [ $i -lt $timeout ]; do > + local new_size=$(stat -c%s "$file" 2>/dev/null || echo 0) > + if [ "$new_size" -gt "$prev_size" ]; then > + return 0 > + fi > + sleep 0.1 > + i=$((i+1)) > + done > + > + echo "logfile not updated within 3 seconds" > + err "$LINENO"
Hmm... not a fan of this open coded "wait for file to change" bash function. This feels like something that a tool can do... (searches) Does inotifywait fit the bill here? https://linux.die.net/man/1/inotifywait > +} > + > start_monitor() > { > logfile=$(mktemp) > $NDCTL monitor -c "$monitor_conf" -l "$logfile" $1 & > monitor_pid=$! > - sync; sleep 3 > + sync > + for i in {1..30}; do > + if ps -p "$monitor_pid" > /dev/null; then > + sleep 0.1 > + break > + fi > + sleep 0.1 > + done > + if ! ps -p "$monitor_pid" > /dev/null; then > + echo "monitor not ready within 3 seconds" > + err "$LINENO" > + fi This does not make sense to. The shell got the pid from the launching the executable. This is effectively testing that bash command execution works. About the only use I can imagine for this is checking that the monitor did not die early, but that should be determined by other parts of the test.
