[Bug testsuite/14574] Add test coverage for stapdyn's runtime

["dsmith at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #10 from David Smith <dsmith at redhat dot com> 2012-09-27 17:48:03 UTC ---
(In reply to comment #9)
> (In reply to comment #8)
> > Commit 8c94efa modifies the systemtap.pass1-4/*.exp tests to also include
> > dyninst.
> > 
> > Most of the cases are modified like this:
> > 
> > ====
> > foreach runtime [get_runtime_list] {
> >     if {$runtime != ""} {
> >     # run test, add '--runtime=$runtime'
> >     } else {
> >     # run test
> >     }
> > }
> > ===
> 
> Must we have the if-else duplicated in every test?  Why can't get_runtime_list
> return the full option already prepared?  If it helps, we could be explicit
> about the default - instead of "" go ahead and spell out "--runtime=kernel".

Originally I had something like the following:

====
foreach runtime [get_runtime_list] {
    if {$runtime != ""} {
      args = "--runtime=$runtime"
    } else {
      args = ""
    }
    # run test, passing $args
}
===

But, I had problems with passing the empty arg. Stap actually got an empty arg,
which confused it.

I went back and forth on whether 'get_runtime_list' should return the full
option. I went the current solution because I append the runtime name to the
test name.  That means you get passes/fails that look like this:

===
FAIL: buildok/string-embedded.stp (dyninst)
FAIL: buildok/system-embedded.stp (dyninst)
===

That output looked a bit better to me, but I could be argued the other way.

As far as doing something like "--runtime=linux", I'm unsure.  One possible
problem here would be with comparing old test results (since we'd be changing
the all the test names).

> Side question: in tapset/ and runtime/ subdirectories I chose linux/ for our
> traditional kernel mode.  Do you think the --runtime option should do the same?
>  Or perhaps both --runtime=kernel and --runtime=linux should be accepted as
> aliases?
> 
> > Each testcase (where it makes sense) will need to be modified to add dyninst
> > support.  Hopefully if we get more runtimes the testsuite will accommodate them
> > much easier.
> > 
> > There are 2 remaining obstacles to getting the testsuite working well with
> > dyninst:
> > 
> > 1) Dyninst outputs some debug messages that interferes with the testsuite
> > output matching.  Lines like:
> > 
> >   Setting bs state to 1
> 
> This line is indeed from dyninst.  It's already been removed upstream, but that
> was after I made my latest rpm snapshot.
> 
> >   found target /usr/lib/libpthread-2.16.so 0x7074
> >   found target /usr/lib/libpthread-2.16.so 0x7985
> >   found target /usr/lib/libpthread-2.16.so 0x7a3d
> >   found target /usr/lib/libpthread-2.16.so 0x6aed
> 
> These are from stapdyn.  I really need to clean these up and hide them behind
> -v flags.  If they're causing testing trouble, that's more reason for me to do
> this sooner than later.

Glad to hear this is being worked on.

> > 2) Dyninst requires a test executable.  Now that probing /bin/true works, this
> > one isn't a "must have", but it would be nice to fix this one.
> 
> Part of the metadata rework I'm doing involves dlopen'ing the module in
> stapdyn.  With that loaded, we could easily call the init/exit functions
> directly when there's no target.  In fact, once we get multiprocess and shared
> memory going, we'll probably always call init/exit this way.
> 
> So while targetless stapdyn has no practical use in general, the sort of toy
> scripts used in testing are important enough that we should make it work.

Glad to hear this is being worked on also.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.