Bug 772180 - [RFE] allow rlAssertRpm to report NVRs of tested packages
Summary: [RFE] allow rlAssertRpm to report NVRs of tested packages
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: beakerlib
Version: rawhide
Hardware: All
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Petr Muller
QA Contact:
URL:
Whiteboard: Release
Depends On: 772178 893064
Blocks: 798718 924763 Fedora-beakerlib-1.9-3 1116317
TreeView+ depends on / blocked
 
Reported: 2012-01-06 10:28 UTC by Martin Cermak
Modified: 2016-09-20 02:08 UTC (History)
5 users (show)

Fixed In Version: beakerlib-1.9-3.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 1108315 (view as bug list)
Environment:
Last Closed: 2014-07-30 06:57:21 UTC
Type: ---


Attachments (Terms of Use)

Description Martin Cermak 2012-01-06 10:28:12 UTC
Description of problem:

   Please, modify the rlAssertRpm function so that it will be able to note the
   NVR of tested package to the beakerlib journal as described in BZ#772178.

Comment 1 Petr Šplíchal 2013-02-14 13:33:00 UTC
Fixing this would resolve version reporting for parametrized tests
cases as well because fix for bug 772622 should be implemented
using rlAssertRpm.

Which reminded me that in the coding style we have the warning
about asserts inside asserts:

    https://fedorahosted.org/beakerlib/wiki/CodingStyle#UsingrlAssertsinrlAsserts

However, for this case I guess using rlAssertRpm is acceptable as
it would produced list of asserts for individual packages which is
exactly what we want.

Comment 3 Martin Cermak 2013-03-21 10:11:48 UTC
Hi, that's good news. I'm currently working (after longer delay) on the expectedness project, because of which this bug was filed. It would be ideal if your patch would work well together with patch that came from Bug 772178 and is already present in production beakerlib.

Which means the package NVR should be reported within <pkgdetails> and </pkgdetails>  in journal.xml as you can see for example in [1]. Thanks.


[1] https://url.corp.redhat.com/abbbec1

Comment 4 Petr Muller 2013-03-25 00:44:31 UTC
Halt for now. Currently I am not convinced this is good approach (=> changing the header). I'll talk to Martin about that. Until that, please do not proceed (coincidentally, I've finally rewritten the journal last week when bored on conference, so I would rather include any change needed in that code directly)

Comment 5 Petr Muller 2013-04-09 14:59:27 UTC
Okay, here's the current (pending update) state:

1) At the time of 'rlJournalStart', package names in PACKAGE, PKGNVR and PACKAGES (the latter two expected to be space-separated) are collected, and recorded. If there are more NVRs for the package name (kernel...), all are recorded. If a package is not present, it is noted too:

$ export PACKAGE=gcc-c++
$ export PKGNVR=bash
$ export PACKAGES="foo beakerlib kernel"
$ rlJournalStart
$ rlJournalPrintText
(...)
:: [   LOG    ] :: Installed:    : gcc-c++-4.7.2-8.fc18.x86_64 
:: [   LOG    ] :: Installed:    : bash-4.2.45-1.fc18.x86_64 
:: [   LOG    ] :: Installed:    : foo is not installed!
:: [   LOG    ] :: Installed:    : beakerlib-1.6.99.2-1.el7.noarch 
:: [   LOG    ] :: Installed:    : kernel-3.8.2-206.fc18.x86_64 
:: [   LOG    ] :: Installed:    : kernel-3.8.4-202.fc18.x86_64 
:: [   LOG    ] :: Installed:    : kernel-3.8.5-201.fc18.x86_64

2) rlAssertRpm logs the versions found, but only in a message:

$ rlAssertRpm kernel
kernel-3.8.2-206.fc18.x86_64
kernel-3.8.4-202.fc18.x86_64
kernel-3.8.5-201.fc18.x86_64
:: [   PASS   ] :: Checking for the presence of kernel rpm
:: [16:57:45] ::  Package versions:
:: [16:57:45] ::    kernel-3.8.2-206.fc18.x86_64
:: [16:57:45] ::    kernel-3.8.4-202.fc18.x86_64
:: [16:57:45] ::    kernel-3.8.5-201.fc18.x86_64
$ rlJournalPrintText 
(...)
:: [   PASS   ] :: Checking for the presence of kernel rpm
:: [   LOG    ] :: Package versions:
:: [   LOG    ] ::   kernel-3.8.2-206.fc18.x86_64
:: [   LOG    ] ::   kernel-3.8.4-202.fc18.x86_64
:: [   LOG    ] ::   kernel-3.8.5-201.fc18.x86_64

Comment 6 Petr Muller 2013-04-09 15:16:18 UTC
I'm not convinced that rlAssertRpm should have any special journal side-effect. 

I know the intention is to have a machine-readable information about the version of the tested package, so we can reason about the expected results, etc. Correct me if I am wrong.

The information collected during rlJournalStart is deemed insufficient, because of the additional environment setup *during* the test. So we need a point in the test when we consider a setup "done" to obtain one definitive information. But I doubt rlAssertRpm should be such point. Actually, if we argue it is OK to mess with packages during the execution, then it is legal to do this:

Phase1 { install foo-123; rlAssertRpm foo; }
Phase2 { test if foo works }
Phase3 { install foo-456; rlAssertRpm foo; }
Phase4 { test if foo works }

What is the desired information about the version of foo? -123 or -456?

My take would be to record the information about the NVRs for each pahse, just as we do it in rlJournalStart. This way you have an information about which the environment at the start of each phase, and you can detect weird cases like the one above. Nothing gets overwritten and lost.

Whadayasay?

Comment 7 Petr Muller 2013-04-09 15:19:33 UTC
Also, for easier processing, it would probably be good to make the XML tag finer:
<pkgdetails name="foo">123.fc666</pkgdetails>, so you can find and match package versions easier.

Comment 8 Martin Cermak 2013-04-10 07:22:19 UTC
(In reply to comment #5)

> $ export PACKAGE=gcc-c++
> $ export PKGNVR=bash
> $ export PACKAGES="foo beakerlib kernel"

I belive just PKGNVR would be sufficient. Most probably some additional marks will 
be needed to record NVRs per test phase (see below), but for the basic functionality 
just PKGNVR seems fine to me.


(In reply to comment #6)
> I'm not convinced that rlAssertRpm should have any special journal
> side-effect. 
> 

My idea behind this is: The whole thing should work with *existing* tests 
(and their respective *existing* metadata in TCMS) without changing them 
in any way. From this point of view using rlAssertRpm is very powerful.

> if we
> argue it is OK to mess with packages during the execution, then it is legal
> to do this:
> 
> Phase1 { install foo-123; rlAssertRpm foo; }
> Phase2 { test if foo works }
> Phase3 { install foo-456; rlAssertRpm foo; }
> Phase4 { test if foo works }
> 
> What is the desired information about the version of foo? -123 or -456?

In order to make a decision about the test result you always need to compare
given acquired NVR with some reference value. The case you showed above is 
really a nonsense as you don't know which one from the two is the given acquired 
NVR. Agreed.

In most cases one single NVR  acquired per the whole test is sufficient. I belive 
this is the case for about 90% of all the tests we currently have in BaseOS. 

But based on recent discussions about the topic I know that some people want
to make a decision also per test phase in which case you'll need one acquired 
NVR per phase (+ somewgere in test metadata ~TCMS you'll need to have reference 
values per phases). For this purpose rlAssertRpm may or may not be suitable.
But this is rather out of the scope of this bug.

Actually my basic idea behind this rfe is to use everything what we have as is.


(In reply to comment #7)

> Also, for easier processing, it would probably be good to make the XML tag finer:
> <pkgdetails name="foo">123.fc666</pkgdetails>, so you can find and match package 
> versions easier.

This is parsable as well :-) But frankly, I don't see much value in it. Currently 
I already have some parser in tcms-results (see bug 924763). It's still in early 
dev phase, so it's still easy to change. But I would love to conclude on some 
reasonable XML structure soon. What about having a lunch or something to discuss?

Comment 9 Petr Muller 2013-04-10 16:44:49 UTC
Okay, we agreed on something like this:

* For expectedness, BeakerLib will track versions of packages in a set of "interesting packages".
* At the start of the test, all packages from PACKAGE, PKGNVR and PACKAGES are in this set.
* At the start of the test, and at the start of each phase, interesting package versions are collected and attached to the currently created header. No earlier collected data will be overwritten.
* When a package presence is tested using rlAssertRpm, it is considered interesting too and added to the set. It any phase started after such rlAssertRpm, its version will be recorded. Records in test headers and previously started phases will be unaffected.

* In the future, assertion records coming from rlRpmAssert will be augmented with the parseable information about found version too.

With the exception of the last point, testing code will be present in my testing beakerlib repository. Last point will be present in the new journal.

Comment 11 Fedora Update System 2014-06-17 13:41:50 UTC
beakerlib-1.9-1.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/beakerlib-1.9-1.fc20

Comment 12 Fedora Update System 2014-06-17 23:29:20 UTC
Package beakerlib-1.9-1.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing beakerlib-1.9-1.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-7442/beakerlib-1.9-1.fc20
then log in and leave karma (feedback).

Comment 13 Fedora Update System 2014-07-02 08:01:31 UTC
beakerlib-1.9-2.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/beakerlib-1.9-2.fc20

Comment 14 Dalibor Pospíšil 2014-07-04 08:50:58 UTC
Please consider the package fixing this bug available in Fedora stable repos once bz1116308 is closed and RHEL stable repos once bz1116317 is closed.

Fixed in:
beakerlib-1.9-2.fc19
beakerlib-1.9-2.fc20
beakerlib-1.9-2.fc21
beakerlib-1.9-3.el5
beakerlib-1.9-2.el6
beakerlib-1.9-2.el7

Comment 15 Fedora Update System 2014-07-17 13:15:49 UTC
beakerlib-1.9-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/beakerlib-1.9-3.fc20

Comment 16 Fedora Update System 2014-07-30 06:57:21 UTC
beakerlib-1.9-3.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.