On Wed, Jan 31, 2024 at 09:06:06 +0100, Ján Tomko wrote:
> On a Tuesday in 2024, Peter Krempa wrote:
> > The function always succeeded and after the removal of programing error
> > checks doesn't even have a 'return false' case. Additionally one of the
> > tests in 'virpcivpdtest' tested that this function never failed on wrong
> > data. Embrace this logic and remove the return value and adjust logging
> > to VIR_DEBUG level to avoid spamming logs.
> >
> > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> > ---
> > src/util/virpcivpd.c | 31 +++++++++++--------------------
> > src/util/virpcivpd.h | 8 +++++---
> > tests/virpcivpdtest.c | 38 ++++++++++++++++++--------------------
> > 3 files changed, 34 insertions(+), 43 deletions(-)
> >
> > diff --git a/tests/virpcivpdtest.c b/tests/virpcivpdtest.c
> > index 20545759d5..a6311bfe76 100644
> > --- a/tests/virpcivpdtest.c
> > +++ b/tests/virpcivpdtest.c
> > @@ -134,10 +130,12 @@ testPCIVPDResourceBasic(const void *data G_GNUC_UNUSED)
> >
> > /* Make sure unsupported RW keyword updates are not fatal. */
> > for (i = 0; i < numUnsupportedCases; ++i) {
> > - if (!virPCIVPDResourceUpdateKeyword(res, false,
> > - unsupportedFieldCases[i].keyword,
> > - unsupportedFieldCases[i].value))
> > - return -1;
> > + /* This test is deliberately left in despite
> > + * virPCIVPDResourceUpdateKeyword always succeeding to prevent
> > + * possible regressions if the function is ever rewritten */
>
> How is this different from any of the pointless tests you removed
> earlier?
Good question. Originally this was the code that reminded me that I
can't just add proper error reporting to that function which I did
originally.
I decided to keep this to preserve this fact in the code, because the
comments/documentation for this particular code are/were in many cases
misleading.
I guess converting this expectation into a comment would be possible,
which would allow us to remove the test. I can do this as a follow up if
you are okay with this solution.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
