Be more careful about default, empty step phases

[happz]

Pull Request Checklist

• implement the feature
• write the documentation
• extend the test coverage
• update the specification
• adjust plugin docstring
• modify the json schema
• mention the version
• include a release note

[happz]

Related to #2827

Very much a work in progress, tests will fail. I'm collecting data and looking for patterns in various use cases that need to be supported, to identify possible conflicts and issues.

[happz]

/packit test

[psss]

Thanks a lot for looking into this! The approach sounds good to me. Added just a couple questions to clarify some points.

[psss]

Perhaps a short explanation why Prepare and Finish steps do not need default phase unless prepare --how or finish --how are provided on the command line would be useful for future readers of the code?

[psss]

Doesn't any provision invocation contains how condition imply that the provision command was provided on the command line and thus the step must be enabled? In other words, shouldn't this be simplified as if self.enabled?

[happz]

Sounds plausible, although I expect it to blow up - if it wouldn't be ruining a test somewhere, I wouldn't end up makingcli_invocations check part of the condition ;)

I'll give it a try because I can't imagine (or recall) how a provision step can have CLI invocation and not be enabled. The self.enabled runs the following:

    def enabled(self) -> Optional[bool]:
        """ True if the step is enabled """
        if self.plan.my_run is None or self.plan.my_run._cli_context_object is None:
            return None

        return self.name in self.plan.my_run._cli_context_object.steps

That looks like you could be right, on the other hand, it's a game of whac-a-mole and I would be very, very surprised if no tests hit this harmless iceberg.

[happz]

And the winner is, tmt try :)

Namely /tests/try/basic:

::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
::   Default Plan
::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::

:: [ 11:05:40 ] :: [  BEGIN   ] :: Running 'TMT_CONFIG_DIR=/tmp/tmp.iZXubjJN2N ./local.exp'
spawn tmt try fedora@local
/var/tmp/tmt/run-053
Let's try /tests/base/bad, /tests/base/good, /tests/base/weird and 3 more tests with /default/plan on fedora@local.

/default/plan
    discover
        how: fmf
        order: 50
        directory: /home/happz/git/tmt/tests/try/basic/data
        summary: 6 tests selected
            /tests/base/bad
            /tests/base/good
            /tests/base/weird
            /tests/core/bad
            /tests/core/good
            /tests/core/weird
    provision
    
        summary: 0 guests provisioned
    prepare
    
        summary: 0 preparations applied
    execute

ESC[31mNo guests available for execution.ESC[0m
send: spawn id exp4 not open
    while executing
"send -- "q\r""
    (file "./local.exp" line 6)
:: [ 11:05:41 ] :: [   FAIL   ] :: Command 'TMT_CONFIG_DIR=/tmp/tmp.iZXubjJN2N ./local.exp' (Expected 0, got 1)
:: [ 11:05:41 ] :: [   PASS   ] :: File '/var/tmp/rlRun_LOG.BX2fu54m' should contain 'Let's try.*/default/plan' 
:: [ 11:05:41 ] :: [   FAIL   ] :: File '/var/tmp/rlRun_LOG.BX2fu54m' should contain 'Run .* successfully finished. Bye for now!' 
::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
::   Duration: 1s
::   Assertions: 1 good, 2 bad
::   RESULT: FAIL (Default Plan)

The provision step is not enabled, but there is a custom CLI invocation, CliInvocation(context=None, options={'image': 'fedora', 'how': 'local'}). Hence the or & test for CLI invocations. self.name in self.plan.my_run._cli_context_object.steps fails because steps is an empty set.

@psss what would you propose? Shall try modify the steps set, to mark steps as enabled?

[LecrisUT]

Doesn't any provision invocation contains how

What I often find confusing is that a default how is injected, usually the virtual provider. It is not well documented how the user can control this default. The only reference I found was of PROVISION_HOW which is specific to tmt's testing

[psss]

Which commands / scenario would actually trigger this code branch? Seems that checks around line 885 always ensure that there are some raw data and they run before wake(). Is it included "just to be sure"?

[happz]

True, _assert_default_phase() may no longer be necessary. I will try it out.