----- Original Message -----
> Hi -
>
> My usual channel for this was unavailable today, so here goes in plain
> email. Took another look at pcpfans.git lberk/papi (up to commit
> 05c56e4f18deaa9). Many good fixes from the previous reviews, thanks!
Thanks for sending this out, its great that these reviews can become
publicly visible (even a summary to the list after internal chat is
highly valuable, if you prefer that channel initially).
> [...]
> First of all, the existing test cases (qa/914 and qa/799) simply must
(I think you mean qa/903 and qa/914)
> stop relying on the -L local context this way. As we discussed on IRC
Add new tests please, as discussed. As stated at the time they were
added, these are simple, directed tests (since we needed *some* form
of test coverage) and the tests exercise specific things (and at no
point did they attempt to verify the values themselves, as of course
they cannot).
They exposed bugs in the authentication mechanism, exercise the install
and remove mechanisms, and are well-suited to some basic pmda-valgrind
exercising, so please keep these in place and now extend with new tests
... thanks.
Having one huge test that attempts to exercise *everything* about any
individual component is usually the wrong way to go - it makes that one
test much slower, and more difficult to debug the test/component when it
does fail (both because its slower *and* because its attempting to do
everything - so, harder to focus in on the one/two aspects its testing,
and its slower when iterating, as one works towards a fix).
> last week, -L makes no sense at all for papi (since it's so stateful;
> each "pmCLIENT -L ..." loads the pmda.so+papi.so to form a new blank
> slate, does one thing, and immediately dies). So no such test will
> ever see counter numbers, or results other than those related to
> exactly one initialization-type operation -- plus any __pmNotifyErr
> noise that would've gone to a pmcd/papi.log file.
That's correct, and well known (they still cover a large amount of the
PMDA functionality). What's your point?...
> In fact, the only way anyone could have ever seen the pmda work and produce
> counter *numbers*, was by -not- using the -L mode, which means its core
> was only ever "hand tested" and qa/914 was to that extent a talisman.
Heh, I see, seeking opportunity to grind the "hand testing" axe.
Contrary to what you say here, the tests exposed a number of bugs & will
continue to do so going forward. They are also small, fast smoke tests
for initial sanity and will be handy for exposing PAPI problems related
to enabling every possible hardware counter at once (i.e. the notorious
short-lived client case).
As discussed awhile back, I added these tests as just a start on testing,
so that we had some basic initial coverage. So, please leave these tests
as-is and build on this base with new tests.
> Anyway, as we were saying on IRC, one plausible way of using -L is via
> dbpmda, instead of a PMAPI client. That could work ... but OTOH might
> be tricky to feed it a fixed dbpmda command script (like a few other
> qa/NNN tests do) and get meaningful answers out, at least because the
> available counters=metrics & runtime environment can so easily vary
> between machines. A simple testcase driver is probably not up to the job.
>
I think you should be able to get by with dbpmda/pmcd and a helper PAPI
test program (ala qa/src/*.c) - you can then check the values the PMDA
gives (via dbpmda/pmcd) against the values a helper test program gives.
As mentioned on IRC, dbpmda and local context opens up valgrind testing
opportunities (highly recommended for this PMDA). For checking actual
values (again, only a tiny part of the the testing needed), using either
pmcd/dbpmda will suit I'm sure.
cheers.
--
Nathan
|