Bug 508650 - [RFE] Conditional phases
Summary: [RFE] Conditional phases
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: beakerlib
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Dalibor Pospíšil
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: Project: Medium, Release
: 1504696 (view as bug list)
Depends On: 1324614
Blocks: 742144
TreeView+ depends on / blocked
 
Reported: 2009-06-29 10:45 UTC by Petr Šplíchal
Modified: 2018-04-16 12:59 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:


Attachments (Terms of Use)
conditional phases (3.63 KB, patch)
2014-03-05 16:03 UTC, Dalibor Pospíšil
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1324533 0 unspecified CLOSED [RFE] add support for reporting that tests were skipped 2021-02-22 00:41:40 UTC

Internal Links: 1324533

Description Petr Šplíchal 2009-06-29 10:45:16 UTC
Quite some time ago we discussed the possibility of phase aborting
and how to implement it. While developing the sanity test suite
for wget I realized that quite often I'd welcome possibility to
run only some of the phases of the suite without need to modify
the code. Consider the following scheme:

    Setup
        HttpTest1
        HttpTest2
        HttpTest3
        HttpTestPerformance
        HttpTestLargeFiles
        FtpTest1
        FtpTest2
        FtpTest3
        FtpTestPerformance
        FtpTestLargeFiles
    Cleanup

The idea of the conditional phases could solve both of these
"features" for us without breaking backward compatibility and
introducing any special syntax or concept (like defining function
for each phase).


Synopsis:

    rlPhaseStartSetup && {
        rlAssertRpm $PACKAGE
        rlRun "TmpDir=\`mktemp -d\`" 0 "Creating tmp directory"
        rlRun "pushd $TmpDir"
    rlPhaseEnd; }

    rlPhaseStartTest Test1 && {
        rlRun "touch foo" 0 "Creating the foo test file"
        rlAssertExists "foo"
        rlRun "ls -l foo" 0 "Listing the foo test file"
    rlPhaseEnd; }

    rlPhaseStartCleanup && {
        rlRun "popd"
        rlRun "rm -r $TmpDir" 0 "Removing tmp directory"
    rlPhaseEnd; }


Implementation:

Quite straightforward: All the rlPhaseStart* functions will return
0 or 1 depending on the conditional logic. This logic could cover
the two most frequent cases: blacklisting/whitelisting and aborts:


Whitelisting/Blacklisting Phases:

Two special variables would be set to whitelist/blacklist specific
phases (this would most likely be an extended regular expression): 

    TEST_PARAM_PHASE_WHITELIST='(Setup|HttpTest.*|Cleanup)'
    TEST_PARAM_PHASE_BLACKLIST='(.*LargeFiles|.*Performance)'

Alias: Run all http test phases, for now skip the time-demanding
large files and performance tests.

[Note that we're using the TEST_PARM prefix to be able to pass
these variables using --test-params="name=value" option of RHTS
workflows.]


Aborting Phases:

There would also be an implicit abort-test-when-setup-failed logic
which would simply skip all the test-type phases if the setup-type
phase fails (i.e. run cleanup-only phases).


The logic:

The complete rlPhaseStart* logic could be something like this:

    # skip blacklisted phases
    if BlackList and PhaseName.matches(BlackList):
        return 1

    # if whitelist set, run whitelisted phases only
    if WhiteList:
        if PhaseName.matches(WhiteList):
            return 0
        else
            return 1

    # run test-type phases only if there is no setup failure
    if PhaseType == Test and journal.setupPhaseResult == FAIL:
        return 1
    else
        return 0

The phase abort logic is quite naive here, yet it can save some
time by skipping the unnecessary tests when setup fails.

To sum up:

    * 100% backward compatible (don't need this, no code change)
    * simple phase abort logic (save time when setup fails)
    * comfortable subtest running / debugging:
      TEST_PARAM_PHASE_WHITELIST='(Setup|Test1|Cleanup)' make run

Perhaps the conditional logic could apply to test-type phases
only, as setup & cleanup are usually the unconditional must-have.
On the other hand I can image a scenario where I want just to run
the setup phase and then do some manual testing... or, vice versa,
just run the specific subtest (in case of manual setup/cleanup).

Ideas? Concerns? Whoopees? ;-)

Comment 1 Ales Zelinka 2009-06-29 11:41:46 UTC
+1 

Really elegant way of phase abortion implementation.

>Perhaps the conditional logic could apply to test-type phases only

I can imagine use cases where it would be useful to skip/run only setup/cleanup phases.

 - running setup phase only to prepare testing environment for a manual testing 
 - ability to run (well-written) unmodified tests in PES by just skipping the setup/cleanup phases

Something for discussion: How do we say a phase is of setup/cleanup/test type? 

By its type? What about non-ABORT setup or non-FAIL test phases (for whatever reason)? 

By the rlPhaseStart+ function it initiated it? How do we deal with rlPhaseStart ABORT Setup?

By its name? rlPhaseStart ABORT i-can-call-my-phases-like-i-want

Comment 2 Petr Muller 2009-06-29 14:02:58 UTC
Hey, this is cool! This finally is a nice way how to implement failed-setup-abort.

About the determining the type (and the 'should I run?' decision logic). For explicit rlPhaseStart{Setup,Test,Cleanup}, things are simple. It's rlPhaseStart which brings problems. I would probably avoid any guesswork about their purpose, and just took a firm stance for these phases as a whole - treat them as a fourth 'custom' type internally. For aborting purposes, I'm for not running these after failed ABORT phase. For *listing, I would propose to allow *listing these just based on name, not by type.

The preferred way is using the rlPhaseStart{S,T,C}.

Comment 7 Fedora End Of Life 2013-04-03 18:56:20 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 9 Dalibor Pospíšil 2014-03-05 16:03:56 UTC
Created attachment 871048 [details]
conditional phases

I have created a patch for this, please look at it if is makes sense. I have done some basic testing on it and it seems to be working.

Comment 11 Dalibor Pospíšil 2014-05-22 15:07:32 UTC
As this change should not have any impact to tests written in the old way I am pushing it to the git without clarification of reporter.

fixed by https://git.fedorahosted.org/cgit/beakerlib.git/commit/?id=13b1cad0d9b4e824c2586311cd670bd308abc48a

Comment 12 Fedora Update System 2014-06-17 13:43:10 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 13 Fedora Update System 2014-06-17 23:30:39 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 14 Fedora Update System 2014-07-02 08:02:53 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 15 Dalibor Pospíšil 2014-07-04 08:50:55 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 16 Dalibor Pospíšil 2014-07-09 13:25:43 UTC
I have found few tasks where PHASES variable name collide, e.g. /distribution/system/setup.
The question is whether we want to fix this in beakerlib by using another variable name or set PHASES and PHASES_BL as reserved names and let others to fix their tasks?
I have no idea how many tasks are impacted but in /distribution there are three of them.

Comment 17 Dalibor Pospíšil 2014-07-09 13:36:58 UTC
The thing is. If the variable PHASES is set the rlPhaseStart does not actually call rljAddPhase if there is no match between phase name and the PHASES regular expression.

Comment 18 Dalibor Pospíšil 2014-07-09 14:16:36 UTC
Or there is really backward compatible solution by actually starting and ending the phase and just skipping its body, like

rlPhaseStart && {
}; rlPhaseEnd

But this would create those phases in journal even if it should be skipped. Such phase would always contain a warning saying that it is not desired to execute the body.

Comment 19 Dalibor Pospíšil 2014-07-17 11:13:17 UTC
We decided to postpone this and think about the concept a little more.

Comment 20 Fedora Update System 2014-07-17 13:16:34 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 21 Fedora Update System 2014-07-19 06:04:10 UTC
Package beakerlib-1.9-3.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-3.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-8522/beakerlib-1.9-3.fc20
then log in and leave karma (feedback).

Comment 22 Fedora End Of Life 2015-01-09 16:12:52 UTC
This message is a notice that Fedora 19 is now at end of life. Fedora 
has stopped maintaining and issuing updates for Fedora 19. It is 
Fedora's policy to close all bug reports from releases that are no 
longer maintained. Approximately 4 (four) weeks from now this bug will
be closed as EOL if it remains open with a Fedora 'version' of '19'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 19 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

Comment 23 Petr Šplíchal 2016-01-04 11:41:32 UTC
As mentioned in comment #19 we need to meet and discuss the idea
in more detail and decide/agree on the best approach. I would
prefer face-to-face discussion rather then lengthy comments.

Do we want to resurrect the concept any time soon? I haven't seen
much interest around this lately. Still it seems to be useful...

Comment 24 Dalibor Pospíšil 2016-04-07 12:30:05 UTC
*** Bug 1324614 has been marked as a duplicate of this bug. ***

Comment 26 Dalibor Pospíšil 2017-10-23 07:49:36 UTC
*** Bug 1504696 has been marked as a duplicate of this bug. ***

Comment 27 Frantisek Sumsal 2018-04-04 11:37:22 UTC
This feature would be really helpful for the recent initiative 'Always Ready RHEL' (not mentioning in z-stream testing in general). Is there any update here?

Comment 28 Dalibor Pospíšil 2018-04-04 11:45:43 UTC
(In reply to Frantisek Sumsal from comment #27)
> This feature would be really helpful for the recent initiative 'Always Ready
> RHEL' (not mentioning in z-stream testing in general). Is there any update
> here?

Unfortunately nothing. We've had other priorities. But I will put this one to our road map.

Comment 29 Michal Sekletar 2018-04-16 12:59:38 UTC
Please consider prioritizing this RFE. Getting it fixed would help us to shorten the time to analyze results of CI runs for systemd z-stream builds.


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