Bug 845054

Summary: [RFE] rlRun should always print out executed command
Product: [Fedora] Fedora Reporter: David Kutálek <dkutalek>
Component: beakerlibAssignee: Dalibor Pospíšil <dapospis>
Status: CLOSED ERRATA QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: azelinka, dkutalek, ksrot, mtruneck, pmuller, psplicha, qa-errata-list
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: Release
Fixed In Version: beakerlib-1.9-3.fc20 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2014-07-30 06:57:15 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1116308, 1116317    

Description David Kutálek 2012-08-01 15:28:18 UTC
Description of problem:

Using comment in rlRun is very usefull to describe what we are running. But it also hides command itself, so we do not see what was actually run. Beakerlib should prin out both in some way.

Version-Release number of selected component (if applicable):

beakerlib-1.5-1.el6eso.noarch

How reproducible:

Always

Steps to Reproduce:
1. eg: rlRun "diff $OldFile $NewFile" 0 "There should be no differences in Files for old and new package"
2.
3.
  
Actual results:

'diff $OldFile $NewFile' is hidden. Therefore author is tempted to leave out a comment and get:
Running 'diff /tmp/blah /tmp/bleh' instead.

Expected results:

E.g. when comment is used, add a LOG entry before PASS/FAIL entry with 'Running...' comment. Or something smarter :-).

Additional info:

Comment 1 Karel Srot 2012-08-01 15:43:32 UTC
If you are going to implement this RFE, please ensure that commands would be printed only in the detailed log, not the summary. It would be very messy, moreover people tend to copy&paste test summary to bugzilla and printed commands would be disturbing (or revealing RH sensitive information, e.g. hostnames, ip adresses,..)

Comment 2 David Kutálek 2012-08-01 15:45:57 UTC
(In reply to comment #1)
> If you are going to implement this RFE, please ensure that commands would be
> printed only in the detailed log, not the summary. It would be very messy,
> moreover people tend to copy&paste test summary to bugzilla and printed
> commands would be disturbing (or revealing RH sensitive information, e.g.
> hostnames, ip adresses,..)

Good point Karel, +1.

Comment 4 Petr Muller 2014-05-20 12:53:25 UTC
*** Bug 1072477 has been marked as a duplicate of this bug. ***

Comment 5 Dalibor Pospíšil 2014-05-20 15:10:57 UTC
Sometimes it is also useful to see the exact beginning of the command. What would you say to following behaviour

# rlRun 'echo a' 0 'print a'
:: [ 17:02:22 ] :: Running 'echo a'
a
:: [   PASS   ] :: print a (Expected 0, got 0)

and also

# rlRun "echo a" 
:: [ 17:06:39 ] :: Running 'echo a'
a
:: [   PASS   ] :: Running 'echo a' (Expected 0, got 0)

Where the fist line would not be included to journal, eg. rlJournalPrintText would not print it out.

It looks strange to me if it would be like:
# rlRun 'echo a' 0 'print a'
a
:: [ 17:02:22 ] :: Running 'echo a'
:: [   PASS   ] :: print a (Expected 0, got 0)

Comment 6 Karel Srot 2014-05-20 15:23:49 UTC
I think that could be address by printing some chars surrounding the output, e.g.

:: [ 17:06:39 ] :: Running 'echo a'
--- last command output start ---
a
--- last command output end ---

Another benefit is that such output won't be mixed with output of other shell commands executed outside of the rlRun.

For me having 
:: [ 17:06:39 ] :: Running 'echo a'
a
:: [   PASS   ] :: Running 'echo a' (Expected 0, got 0)
would be confusing because I might expect (after a first sight) that the command was executed twice.

Comment 7 Ales Zelinka 2014-05-20 17:11:27 UTC
(In reply to Dalibor Pospíšil from comment #5)
> Where the fist line would not be included to journal, eg. rlJournalPrintText
> would not print it out.

That would kind of defeat the purpose of journal as a machine readable _structured_ record of the test execution..

AFAIK if the executed command is important (and the existence of this bug implies it is) then it should be stored in the journal.

And if it's related to the assert (rlRun) it should be stored in the rlRun's results, not as an independent log message.

I suggest to extend the 'test' journal element like this:
<test message="comment (Expected 1, got 1)" command="some command">PASS</test>

And then simply skip printing the command attribute's content in rlJournalPrintText if you feel like doing so (and it already has a --full-journal parameter - that can be used to switch command printing on)

As for the syntax of the stdout logs: I prefer the 1st variant with 

 - prefix
 - command's stdout/err
 - suffix

Because if nicely frames the rlRun's output

Comment 8 Petr Muller 2014-05-21 09:38:46 UTC
(In reply to Ales Zelinka from comment #7)
> (In reply to Dalibor Pospíšil from comment #5)
> > Where the fist line would not be included to journal, eg. rlJournalPrintText
> > would not print it out.
> 
> That would kind of defeat the purpose of journal as a machine readable
> _structured_ record of the test execution..
> 
> AFAIK if the executed command is important (and the existence of this bug
> implies it is) then it should be stored in the journal.
> 
> And if it's related to the assert (rlRun) it should be stored in the rlRun's
> results, not as an independent log message.
> 
> I suggest to extend the 'test' journal element like this:
> <test message="comment (Expected 1, got 1)" command="some
> command">PASS</test>

I support this view.

Comment 10 Dalibor Pospíšil 2014-05-21 15:12:57 UTC
Now to the printed output. I don't like to print out too many unnecessary lines. I would like to use current :: [ PASS/FAIL ] :: a little modififed tag as the ending one and for the beginnig I would like to keep the standard prefix. So my proposal is

# rlRun "echo a" 
:: [  BEGIN   ] :: Running 'echo a'
a
:: [   PASS   ] :: Command 'echo a' (Expected 0, got 0)


# rlRun "echo a" 0 "print a"
:: [  BEGIN   ] :: print a :: actually running 'echo a'
a
:: [   PASS   ] :: print a (Expected 0, got 0)

Comment 11 Ales Zelinka 2014-05-21 15:54:32 UTC
Can you please have the command in the same spot in both versions? For example
:: [  BEGIN   ] :: Running 'echo a'
and 
:: [  BEGIN   ] :: Running 'echo a' Comment 'comment'

Just to make both look more similar (I known my brain/parser would appreciate it)

Thanks!

Comment 12 Dalibor Pospíšil 2014-05-22 08:37:52 UTC
I don't think it is necessary as the result messages already differ but everything is possible. I just wanted both versions to be similar in their BEGIN and result messages so it can be easily identified as a pair messages.

Your variant would be composed differently for BEGIN and result in the commented version.

I mean this:
:: [  BEGIN   ] :: print a :: actually running 'echo a'
a
:: [   PASS   ] :: print a (Expected 0, got 0)

vs.

:: [  BEGIN   ] :: Running 'echo a' :: comment 'print a'
a
:: [   PASS   ] :: print a (Expected 0, got 0)


Finally it seems there are no objections to the concept except the order of printed information.

Comment 13 Ales Zelinka 2014-05-22 09:26:39 UTC
you make a valid point. it;'s a matter of preference and only a very minor issue for me. Hence I deffer to my usual MO:

whoever writes the code gets to decide

Comment 15 Fedora Update System 2014-06-17 13:41:41 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 16 Fedora Update System 2014-06-17 23:29:12 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 17 Dalibor Pospíšil 2014-06-18 13:15:32 UTC
There is an issue if IFS is set to something else that ' ' (space). An option --command given to beakerlib-journalling is messed up. The solution is to use eval to the whole call.

Comment 19 Fedora Update System 2014-07-02 08:01:22 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 20 Fedora Update System 2014-07-03 04:02:56 UTC
Package beakerlib-1.9-2.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-2.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-7998/beakerlib-1.9-2.fc20
then log in and leave karma (feedback).

Comment 21 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 22 Fedora Update System 2014-07-17 13:15:43 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 23 Fedora Update System 2014-07-30 06:57:15 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.