Re: rpmguard-integration branch available

[Kamil Paral]

----- "James Laska" <jlaska(a)redhat.com&gt; wrote:

>  Nice work Kamil, this looks great.  I don't mind the output
> format to
>  be
>  honest, but I think the discussion you and Will had during the recent
>  QA
>  meeting [1] about collapsing results from different architectures
>  together makes sense.  We probably want to err on the side of
>  displaying
>  less information for developer review than scaring them away with
>  tons
>  of repeated information.  Besides, we can scare them later by just
>  providing the results against their builds :) 
Already as a ticket:
https://fedorahosted.org/autoqa/ticket/94

I want to have it implemented this week. Good idea, Will.

>  
>  I don't think this is specific to the rpmguard test, but it's a
>  little
>  confusing for me (not knowing the internals) to see two different
>  scripts 'rpmguard' and 'rpmguard.py' in the test directory.  Looking
>  further, I believe rpmguard.py provides the class that autotest calls
>  from the provided control file and is intended to be imported only. 
>  Is
>  that correct? 
Actually, I think it's rather specific to rpmguard test. In other tests
you usually use some binary from fedora repos located in system paths
(you just do 'yum install foo' at setup), so we can afford to have test
named same as the binary (e.g. rpmlint binary and rpmlint test).

For rpmguard it got a little complicated, because I already develop
the rpmguard in the same directory (and we don't have it packaged
or something). And because the wrapper must be called <test>.py, I
renamed rpmguard.py to rpmguard and created the wrapper with the prior
name.

There are a few solutions:
1. change test naming conventions
2. rename the rpmguard test
3. move rpmguard upstream to another folder 
4. move rpmguard upstream to its own project (overkill?)

Third option is probably the easiest. What you think? Just tell me
where would you prefer to put it so we won't make mess of the project
structure.

>  
>  Some other thoughts ...
>  
>        * I like how you're using the setup() method and checking for
>          specific versions of required software.  I imagine this might
>          become a common thing, I wonder if in the future we could
>  offer
>          a common require_version() method in the autoqa base package
>        * In the initialize() method ... should we look into using
>  mktemp
>          (or similar) here instead of os.makedirs?  Could multiple
>  runs
>          of the test pick-up stale data? 
I have to admit cheating, I just copied that stuff from Will's rpmlint
test :) I haven't studied if we have some conventions where to save data
etc. So maybe Will could better answer that. Surely I don't need the
data to be persistent in this case so mktemp works well for me. Multiple
runs should work ok in current implementation, because the urlgrabber
seems to be overwriting existing files.


>        * In run_once(), I like that you are displaying errors on
>  stdout
>          as well as in the log.  I wonder if we could rely on the
>  python
>          logging module (or a common autoqa logging subclass to
>  provide
>          proper format and loglevels) to send results to multiple
>  places?
>          The trick I think will be writing this such that it can still
>          run in stand-alone mode.  Looks like Lucas and Michael are
>  doing
>          similar things with integrating the KVM tests into upstream
>          autotest [2].  Is this something we could make use of as
>  well?
>  
>  [1]
>  https://fedoraproject.org/wiki/QA/Meetings/20091214#rpmguard_integration
>  [2] http://patchwork.kernel.org/patch/40190/ 
Some kind of logging would be nice, I was also thinking about this
when I printed the messages both to stdout and results. Now when
I want to reformat the rpmguard test output to eliminate duplicities
I am not sure if I would use it (the results output will be probably
different from stdout), but for most tests this could simplify 
things.

I hope our one and only autoqa guru wwoods can tell us more about this
topic :)

>  
>  _______________________________________________
>  autoqa-devel mailing list
>  autoqa-devel(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/autoqa-devel

rpmguard-integration branch available

[James Laska]

On Tue, 2009-12-15 at 16:03 -0500, Kamil Paral wrote:
>  ----- "James Laska" <jlaska(a)redhat.com&gt; wrote:
>  
>  > Nice work Kamil, this looks great.  I don't mind the output format to
>  > be
>  > honest, but I think the discussion you and Will had during the recent
>  > QA
>  > meeting [1] about collapsing results from different architectures
>  > together makes sense.  We probably want to err on the side of
>  > displaying
>  > less information for developer review than scaring them away with
>  > tons
>  > of repeated information.  Besides, we can scare them later by just
>  > providing the results against their builds :)
>  
>  Already as a ticket:
>  https://fedorahosted.org/autoqa/ticket/94
>  
>  I want to have it implemented this week. Good idea, Will.
>  
>  > 
>  > I don't think this is specific to the rpmguard test, but it's a
>  > little
>  > confusing for me (not knowing the internals) to see two different
>  > scripts 'rpmguard' and 'rpmguard.py' in the test directory. 
> Looking
>  > further, I believe rpmguard.py provides the class that autotest calls
>  > from the provided control file and is intended to be imported only. 
>  > Is
>  > that correct?
>  
>  Actually, I think it's rather specific to rpmguard test. In other tests
>  you usually use some binary from fedora repos located in system paths
>  (you just do 'yum install foo' at setup), so we can afford to have test
>  named same as the binary (e.g. rpmlint binary and rpmlint test).
>  
>  For rpmguard it got a little complicated, because I already develop
>  the rpmguard in the same directory (and we don't have it packaged
>  or something). And because the wrapper must be called <test>.py, I
>  renamed rpmguard.py to rpmguard and created the wrapper with the prior
>  name.
>  
>  There are a few solutions:
>  1. change test naming conventions
>  2. rename the rpmguard test
>  3. move rpmguard upstream to another folder 
>  4. move rpmguard upstream to its own project (overkill?)
>  
>  Third option is probably the easiest. What you think? Just tell me
>  where would you prefer to put it so we won't make mess of the project
>  structure. 
I see now, okay it makes sense to me.  Option#3 isn't bad, just extra
overhead on your part.  I'm comfortable leaving things as is for now,
but wouldn't object if you'd like to give rpmguard it's own upstream.
Mainly, I just wasn't clear on the difference ... and forgot which
module really gets called by run_test method in the 'control' file.
 
>  > Some other thoughts ...
>  > 
>  >       * I like how you're using the setup() method and checking for
>  >         specific versions of required software.  I imagine this might
>  >         become a common thing, I wonder if in the future we could
>  > offer
>  >         a common require_version() method in the autoqa base package
>  >       * In the initialize() method ... should we look into using
>  > mktemp
>  >         (or similar) here instead of os.makedirs?  Could multiple
>  > runs
>  >         of the test pick-up stale data?
>  
>  I have to admit cheating, I just copied that stuff from Will's rpmlint
>  test :) I haven't studied if we have some conventions where to save data
>  etc. So maybe Will could better answer that. Surely I don't need the
>  data to be persistent in this case so mktemp works well for me. Multiple
>  runs should work ok in current implementation, because the urlgrabber
>  seems to be overwriting existing files.
>  
>  
>  >       * In run_once(), I like that you are displaying errors on
>  > stdout
>  >         as well as in the log.  I wonder if we could rely on the
>  > python
>  >         logging module (or a common autoqa logging subclass to
>  > provide
>  >         proper format and loglevels) to send results to multiple
>  > places?
>  >         The trick I think will be writing this such that it can still
>  >         run in stand-alone mode.  Looks like Lucas and Michael are
>  > doing
>  >         similar things with integrating the KVM tests into upstream
>  >         autotest [2].  Is this something we could make use of as
>  > well?
>  > 
>  > [1]
>  > https://fedoraproject.org/wiki/QA/Meetings/20091214#rpmguard_integration
>  > [2] http://patchwork.kernel.org/patch/40190/
>  
>  Some kind of logging would be nice, I was also thinking about this
>  when I printed the messages both to stdout and results. Now when
>  I want to reformat the rpmguard test output to eliminate duplicities
>  I am not sure if I would use it (the results output will be probably
>  different from stdout), but for most tests this could simplify 
>  things.
>  
>  I hope our one and only autoqa guru wwoods can tell us more about this
>  topic :) 
I played around with a quick'n'dirty patch that uses the logging module
(which autotest extends for us).

http://pastie.org/747317

It's definitely not perfect or something I would consider committing.  I
just was curious how the logging module would work with our test
wrappers.  The nice thing is autotest splits the output based on
loglevel for us ...

        # ls /usr/share/autotest/client/results/post-koji-build
        \:rpmguard.x86_64/rpmguard/debug/
        rpmguard.DEBUG  rpmguard.ERROR  rpmguard.INFO  rpmguard.WARNING
        
        # cat /usr/share/autotest/client/results/post-koji-build
        \:rpmguard.x86_64/rpmguard/debug/rpmguard.ERROR 
        rpmlint.noarch                                           | 348
        kB     00:00 ...     ]  0.0 B/s |  53 kB     --:-- ETA 
        12/17 11:15:16 ERROR|  rpmguard:0124| added        REQUIRES
        libportaudio.so.2  
        removed      REQUIRES libpulse.so.0  
        removed      REQUIRES libpulse.so.0(PULSE_0)  
        12/17 11:15:24 ERROR|  rpmguard:0124| added        REQUIRES
        libportaudio.so.2()(64bit)  
        removed      REQUIRES libpulse.so.0()(64bit)  
        removed      REQUIRES libpulse.so.0(PULSE_0)(64bit)  
        12/17 11:15:27 ERROR|  rpmguard:0124| added        REQUIRES
        libportaudio.so.2()(64bit)  
        removed      REQUIRES libpulse.so.0()(64bit)  
        removed      REQUIRES libpulse.so.0(PULSE_0)(64bit)  
        12/17 11:15:34 ERROR|  rpmguard:0124| added        REQUIRES
        libportaudio.so.2  
        removed      REQUIRES libpulse.so.0  
        removed      REQUIRES libpulse.so.0(PULSE_0)  

The format of the logs is something autotest has defined, and we can
override if desired.  Also, in this example I was assuming that any
output from rpmguard was considered a failure.  So that output was sent
using logging.error(...).

The logging module is pretty flexible.  It's easy to add additional
logging destinations (files, stdout etc...), each with their own output
formats.  So we could continue creating the single rpmguard.log results
file if desired.  In fact, we could possible have this done for every
test by providing an AutoQA specific output handler in addition to what
autotest has defined.

Thoughts/comments/concerns?

Thanks,
James

rpmguard-integration branch available

[Kamil Paral]

----- "Kamil Paral" <kparal(a)redhat.com&gt; wrote:

>  ----- "James Laska" <jlaska(a)redhat.com&gt; wrote:
>  
>  > Nice work Kamil, this looks great.  I don't mind the output format
>  to
>  > be
>  > honest, but I think the discussion you and Will had during the
>  recent
>  > QA
>  > meeting [1] about collapsing results from different architectures
>  > together makes sense.  We probably want to err on the side of
>  > displaying
>  > less information for developer review than scaring them away with
>  > tons
>  > of repeated information.  Besides, we can scare them later by just
>  > providing the results against their builds :)
>  
>  Already as a ticket:
>  https://fedorahosted.org/autoqa/ticket/94
>  
>  I want to have it implemented this week. Good idea, Will. 
Fix committed. It's not very pretty hack, it's going to be smoothed out
in the future (with logging improvements etc).