Bug 843823

Can you please describe the goal in more detail? What problem does this RFE address? 
I'm afraid that adding another layer of documentation (on top of PURPOSE file, assert comments and classic bash comments) will increase test maintenance costs.

When going through TCMS plan runs, all the automated tests has empty fields Setup, Breakdown, Actions, Expected results. And no Notes, which we so far use for displaying PURPOSE of the test.

I was wondering, why we have there blank spaces and wether it would make sense to be able to read there what the test actually does.

Not sure about mentioned overhead, proposed comments would still be bash comments, just with a keyword and tag. Looking at the example tcms case created, I would say these taged comments should be very high level and just a few in whole test.

Btw, some people do not use assert comments any more, because they are obfuscating actuall command in log, making investigation harder.

(In reply to comment #1)
> Can you please describe the goal in more detail? What problem does this RFE
> address? 
> I'm afraid that adding another layer of documentation (on top of PURPOSE
> file, assert comments and classic bash comments) will increase test
> maintenance costs.

Hi,
I would like to suggest use some shorter variant "# bkrdoc:" is really long fom my point of view:
for example it count be "#B " or "##" or "###"
   Thanks Honza

Perhaps we could enforce some comment being present per phase (enforce = have some way how to detect or parse if all phases have such comments), and consider such test well-documented. Combined with few inline annotated comments, it should be enough to describe on high-level what the test does.

(In reply to comment #1)
> Can you please describe the goal in more detail? What problem does this RFE
> address? 
> I'm afraid that adding another layer of documentation (on top of PURPOSE
> file, assert comments and classic bash comments) will increase test
> maintenance costs.

It surely will, as we do not currently document our tests at all. I think we should go for minimal amount of added work, and get the best docs we can. The problem is that the the categories you have stated all serve different purpose. This does not overlap with PURPOSE or normal comments (they serve different purpose), and overlaps partially with assert comments, but I do not know how to fully utilize these.

(In reply to comment #4)
> Perhaps we could enforce some comment being present per phase (enforce =
> have some way how to detect or parse if all phases have such comments), and
> consider such test well-documented. Combined with few inline annotated
> comments, it should be enough to describe on high-level what the test does.

Vow, lets make phase without comment fail! ;-)
Seriously, nice idea. Perhaps some lint-like check in make bkradd?

> (In reply to comment #1)
> > Can you please describe the goal in more detail? What problem does this RFE
> > address? 
> > I'm afraid that adding another layer of documentation (on top of PURPOSE
> > file, assert comments and classic bash comments) will increase test
> > maintenance costs.
> 
> It surely will, as we do not currently document our tests at all. I think we
> should go for minimal amount of added work, and get the best docs we can.
> The problem is that the the categories you have stated all serve different
> purpose. This does not overlap with PURPOSE or normal comments (they serve
> different purpose), and overlaps partially with assert comments, but I do
> not know how to fully utilize these.

As to assert comments:

In cases it makes sense to have same comment for runtime (assert comment) and for high-level documentation (bkrdoc), we should be able to not duplicate it like this:

rlRun "diff /tmp/old-testsuite /tmp/new-testsuite" 0 "No testsuite regressions present" # bkrdoc @expected

Automation tool could be imho smart enough to harvest assert comment here. 

I filed relevant bug, because people have to decide now regarding assert comments: to use comment, or see what command was run:
https://bugzilla.redhat.com/show_bug.cgi?id=845054

Hi, thanks for suggestion. We may consider it, especially more hashtags are very quickly written. However, we have to double check that such comments are not present massively in existing tests, to not mix comments written for different purpose.

(In reply to comment #3)
> Hi,
> I would like to suggest use some shorter variant "# bkrdoc:" is really long
> fom my point of view:
> for example it count be "#B " or "##" or "###"
>    Thanks Honza

Nice idea, David! +1 for keeping the test documentation as near to
the code as possible and prevent duplication / separation with the
PURPOSE. Also encouraging to have at least short phase description
seems to me to be useful.

Brainstorming about the format: What about rather using postfix
for both interesting asserts and other comments. Looking at the
example above I find the prefix a bit disturbing while reading the
content. Perhaps something likes this?

> # On client, prepare squid for caching yum repositories [setup]
> rlPhaseStartSetup "Client setup"
>     # Enable SELinux boolean, allow to connect from any host [setup]
>     rlRun "setsebool squid_connect_any on" 0 "Enable squid SELinux boolean"
>     rlRun "rlFileBackup $SquidConf" 0 "Backing up $SquidConf"
>     rlRun "sed -i 's/allow localhost/allow all/' $SquidConf" \
>             0 "Allowing to access squid from any host"
>     rlRun "sed -i 's/^#cache_dir/cache_dir/' $SquidConf" \
>             0 "Enabling the cache_dir"
>     rlRun "rlServiceStart squid"
>
>     # Fetch CA certificate, add to CA bundle [setup]
>     rlRun "rhts-sync-block -s READY $SERVERS" 0 "Waiting for the server"
>     rlRun "wget http://$SERVERS/repo/ca.crt" 0 "Fetching CA certificate"
>     rlRun "cat ca.crt >> $CaBundle" \
>             0 "Appending CA certificate to CA bundle"
>
>     # Set up yum repositories for ftp, http and https protocol [setup]
>     rlRun "rlFileBackup --clean /etc/yum.repos.d" \
>             0 "Backing up yum repositories"
>     for protocol in "ftp" "http" "https" ; do
>         cat > /etc/yum.repos.d/$protocol-test.repo <<-EOF
>             [$protocol-repo]
>             name=Proxy test repo for the $protocol protocol
>             baseurl=$protocol://$RepoUrl
>             enabled=1
>             gpgcheck=0
>             EOF
>         rlAssert0 "Adding the $protocol-test repository" $?
>     done
>     rlRun "yum --noplugins repolist" \
>             0 "Check that repositories are working correctly" # [setup]
> rlPhaseEnd

Which could result in something like the following:

> On client, prepare squid for caching yum repositories
>  * Enable SELinux boolean, allow to connect from any host
>  * Fetch CA certificate, add to CA bundle
>  * Set up yum repositories for ftp, http and https protocol
>  * Check that repositories are working correctly

BTW/OFFTOPIC, shouldn't we track non-internal BeakerLib bugs in Fedora?
https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&component=beakerlib

I would prefer using one of the existing documentation extraction tools, rather than brewing a new one.

(In reply to comment #7)

> BTW/OFFTOPIC, shouldn't we track non-internal BeakerLib bugs in Fedora?
> https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&component=beakerlib

I do not mind discussing it here.

Can you name some of these, so we can evaluate whether it will fit?

(In reply to comment #8)
> I would prefer using one of the existing documentation extraction tools,
> rather than brewing a new one.

(In reply to comment #9)
> Can you name some of these, so we can evaluate whether it will fit?

I'm afraid I cannot: I'm not really an expert in this. We are using POD for documenting bash files in BL itself.

I think this deserves a small workgroup and be implemented as a small, properly led project: evaluating the requirements, researching possible solutions, choosing one that suits the reqs...

David, care to pick up that torch?

Yep, though it must wait a little now. Will let you know when ready.

Just for inspiration see the testimony project on github. Seems
guys developing the tool (intended for python code) have a similar
motivation:

    Are you tired of managing your test cases in a test case
    management tool and your test code in a python automation
    framework? Testimony can help to use the python automation
    framework as a test case repository tool.

    https://github.com/sthirugn/testimony

Keeping the test code and manual test case description at the same
place really makes sense. And if necessary the documentation can
be synced to the test case management system.

Another inspiration from the Doxygen:

  # @setup This is a setup step comment 
  # @action This is an action step description
  # @result Describing expected result
  # @cleanup And do not forget to clean up after yourself!

http://en.wikipedia.org/wiki/Doxygen

Ad Doxygen, probably with doubled hash as with their examples to also provide a way for general comment?

## For each reproducer given
for REPR in REPRS; do

    ## @action Run reproducer
    rlRun "$REPR" 0

    ## @action Check it didn't crash
    rlRun ...
    rlRun ...


Besides syntax itself, what structure do we want? 

Ie. eg. follow existing tcms doc fields (setup, breakdown, actions, results)?
I would say lets be compatible with existing tcms, but not stick with it.

Following phases structure may be tempting but phases may be conditioned or created in a loop...

Also maintaining pairs of action-result is somehow questionable. For a lot of cases like one above it would be probably ok to have just action. Therefore empty results have to be used when encountering another action prior to result.

What's the difference between:

> ## @action Run reproducer
> rlRun "$REPR" 0

and 

> rlRun "$REPR" 0 "Run reproducer"

? (except for the obvious one - syntax).

> rlRun "$REPR" 0 "Run reproducer"

Only "Run reproducer" used to be printed.

In the first case text goes to test docs, in the second text goes to test output/journal. Two different purposes.

Doxygen has something like

rlRun "$REPR" 0 "Run reproducer" #< @action

which tells doc processor to grab text for @action from preceding command/statement.

But with this construct you aim one text for two purposes and have to thing more about it to make sense for both. In this simple example it would work nicely. Once 'run reproducer' consists of eg. two rlRun statements it probably wouldn't.

got it (I think): it was an example illustating possible syntax, not an example of real life bkrdoc usage. Please carry on.

I have talked today to a student who might be interested in solving this in RH Lab. I have sent David Kutalek a contact and introduction so he can initiate some coop.

This package has changed ownership in the Fedora Package Database.  Reassigning to the new owner of this component.

Student Jiri Kulda has prepared a draft for tagging format and a generator to harvest those docs out from it. It is published on GitHub:

https://github.com/rh-lab-q/bkrdoc/tree/master/Generator

There are some additional request that documentation should have two parts: purpose and steps. Purpose should be located in the top part of the test, while steps should be placed through the whole test code.


It seems to me that with respect to proposed syntax [1] we could use for purpose either first tagged block like:

#!/bin/bash

# license and whatever

#@ 
# Checks that all basic wget download functionality works.
# Which means test various protocols and a few associated functions,
# against httpd server the test setup. So also a little httpd smoke.

OR use explicit PURPOSE tag there (so first tag can be something else) like:

#!/bin/bash

# license text

#@ @LICENSE gpl
#@ @AUTHOR hero

#@ @PURPOSE
# Checks that all basic wget download functionality works.
# Which means test various protocols and a few associated functions,
# against httpd server the test setup. So also a little httpd smoke.

Note that the meaning of individual tags (like @PURPOSE, @action, @LICENSE etc) is to be discussed - initial idea was to be able to be able to 'type' individual docs string for some reason. Like:

#@ @action Setup httpd server for http and https
... code
#@ @result Httpd server listens on ports http/80, https/443.

All in all, we need to discuss more - please provide a feedback if you can. Thanks.

The Generator

The Generator is made but, it need a lot of testing. So if you find somu bug, please contact me and I'll try to fix it.

Is there something what is going to be delivered within beakerlib package? What are the plans?

(In reply to Dalibor Pospíšil from comment #26)
> Is there something what is going to be delivered within beakerlib package?
> What are the plans?

Good question. I think it would be nice to integrate bkrdoc
into beakerlib itself, at least on documentation level.

What about at least mentioning bkrdoc tool in 'man beakerlib',
with link to the project?

That's the easiest way. Can you or the author propose a patch for beakerlib documentation?

Jiri, would you mind writing the patch as discussed above?
I am willing to review it.

Yes, I will look at it. 
I will let you know when I will have something.

Created attachment 1203611 [details]
Patch for beakerlib man page with added information about bkrdoc

Patch for BeakerLib man page with added information about bkrdoc tool.

Created attachment 1205879 [details]
Patch for beakerlib man page for beakerlib.sh

fixed by https://git.fedorahosted.org/cgit/beakerlib.git/commit/?id=960f3ec762ab15247049dcbd785ff9f549627fe4