On Wed, Aug 13, 2025 at 06:28:14PM -0700, Marc Herbert wrote:
> Reviewing only the shell language part, not the CXL logic.
>
> On 2025-08-04 02:01, alison.schofi...@intel.com wrote:
>
> > +# Allow what shellcheck suspects are unused - the arrays
> > +# shellcheck disable=SC2034
>
> That's very indiscriminate and shellcheck -x is usually pretty good
> there... did you use -x?
Yes. Shellcheck -x doesn't ignore SC2034.
>
> Alternatively, could you place this only before the arrays that
> shellcheck gets wrong for some reason? (which reason?)
I considered and chose the global disable for the arrays and commented
same. The syntax is valid bash syntax that shellcheck has not caught up
to understanding yet.
There is always this option to see what may be masked by shellcheck
disables:
grep -v -E '^\s*#\s*shellcheck\s+disable=' cxl-translate.sh | shellcheck -x -
>
>
> > +
> > +check_dmesg_results() {
> > + local nr_entries=$1
> > + local expect_failures=${2:-false} # Optional param
> > + local log nr_pass nr_fail
> > +
> > + log=$(journalctl -r -k --since "$log_start_time")
>
> -r is IMHO not a very common option:
I'll expand both -r and -k options.
>
> log=$(journalctl --reverse -k --since "$NDTEST_START")
>
>
> > + nr_pass=$(echo "$log" | grep -c "CXL Translate Test.*PASS") || nr_pass=0
> > + nr_fail=$(echo "$log" | grep -c "CXL Translate Test.*FAIL") ||
> > nr_fail=0
>
> Not sure about reading the entire log in memory. Also not sure about
> size limit with variables... How about something like this instead:
This is reading the dmesg log per test_table or test_sample_set.
There are currently 6 data sets that are run thru the test and results
checked per data set. Each set has an expected number of PASS or FAIL.
I'm pretty sure any one log is much smaller than any log we typically
generate just by loading the cxl-test module in other cxl-tests.
I don't expect to use the general check_dmesg() here because anything
this test wants to evaluate is in the logs it reads after each data
set.
>
> local jnl_cmd='journalctl --reverse -k --grep="CXL Translate Test"
> --since '"$NDTEST_START"
> local nr_pass; nr_pass=$($jnl_cmd | grep 'PASS' | wc -l)
> local nr_fail; nr_pass=$($jnl_cmd | grep 'FAIL' | wc -l)
>
>
> > + if [ "$expect_failures" = "false" ]; then
>
> if "$expect_failures"; then
Is the existing pattern wrong or is that an ask for brevity?
Thanks for the review!
>
