test suite reviews and discussions
 help / color / mirror / Atom feed
From: Patrick Robb <probb@iol.unh.edu>
To: David Marchand <david.marchand@redhat.com>
Cc: dts@dpdk.org, Thomas Monjalon <thomas@monjalon.net>,
	Aaron Conole <aconole@redhat.com>
Subject: Re: [PATCH] tests/TestSuite_crypto_perf_cryptodev_perf.py Improving lscpu parsing
Date: Mon, 16 Sep 2024 23:36:14 -0400	[thread overview]
Message-ID: <CAJvnSUAq8Q1LJu0Y6JYOZ-vp1QZVytDU8vE0Mb1Zprm6wX6xOQ@mail.gmail.com> (raw)
In-Reply-To: <CAJFAV8yZxrB0=tu-Lu3XJw+R12odBXVXa=yi3XUrrVLjoTuhaA@mail.gmail.com>

On Mon, Sep 16, 2024 at 3:17 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, Sep 16, 2024 at 6:27 AM Patrick Robb <probb@iol.unh.edu> wrote:
> >
> > This testsuite fails to parse lscpu output on some systems because
> > it expects that there be no leading whitespace, which is not
> > true on all systems.
>
> The leading whitespace is a generic issue, not really related to "some" systems.
> The issue is that keys of interest for the rest of the code ('Core(s)
> per socket', 'Socket(s)', 'CPU(s)') may have leading whitespace on
> some systems.

^The above is what I had in mind when writing my commit - your wording
gets to the point better, thanks.

I had observed this issue in trying to run the testsuite on an ARM
Neoverse N-1 Ampere server at UNH, and noted that the leading
whitespace on it's lscpu output was preventing the 'Core(s) per
socket' key from appearing in the output dict (at least without the
trimming which this commit provides).

>
> Could you detail which systems have such issue?
>

From a quick look through of our systems I think that there appears to
be a mixture of systems with lscpu output with no leading whitespace
before any of the keys, and others (like the Ampere server I
mentioned) which do have that leading whitespace/indentation. I'm not
sure what the pattern is (I guess it probably depends on the OS on the
system), and a google search didn't seem to reveal much about this
question. However, from looking at the lscpu man page, I do see that
there is a --json flag which you can include.

So, in my opinion the ideal way to deal with these situation where you
must read some information from the system, is to use the options
available (like lscpu --json) which are guaranteed to return a machine
readable output. Getting into the business of reading text output like
this is not ideal, given that it is subject to change in the future.
But, I have the same view as you that this commit is not likely to
break this testsuite for any systems which are running the suite
currently, it will only help for systems with the indented lscpu
output.

  reply	other threads:[~2024-09-17  3:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16  4:26 Patrick Robb
2024-09-16  7:16 ` David Marchand
2024-09-17  3:36   ` Patrick Robb [this message]
2024-09-18  7:01     ` David Marchand
2024-09-18  7:16   ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJvnSUAq8Q1LJu0Y6JYOZ-vp1QZVytDU8vE0Mb1Zprm6wX6xOQ@mail.gmail.com \
    --to=probb@iol.unh.edu \
    --cc=aconole@redhat.com \
    --cc=david.marchand@redhat.com \
    --cc=dts@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).