how will the CI system know pep8 is installe don the system, should we not install it?

@psss maybe it would be wise to add some installation step in the prepare section? just thinking

Hm, good question. Maybe we could make sure some essential tools for testing are installed on the system (I think pep8 checker could be among those). However, for more advanced use cases we should advice using a prepare section config for installing additional dependencies.

I'd be very conservative about a "shared" expectation of what's supposed to be pre-installed on the SUT. The discovery/execute pair should have few things to say when it comes to SUT preparation, exactly for this use case - if one uses (hypothetical) pytest tool, the CI system should take care of installing PyTest before moving on, pep8 could become a tool on its own. I'm not sure what's better, the list of preinstalled apps shell could use seems to me as way too easy to extend and abuse.

Thanks for the feedback. Makes sense. We should keep the list of pre-installed packages as small as possible. Or perhaps reasonably small? But there should also be some very easy way how to list required packages as this is quite common use case, I guess. It does not make sense to write an ansible playbook for a single required package such as python3-pep8. Perhaps something like this?

/pull-request:
    prepare:
        tool: dnf
        install: python3-pep8
    execute:
        tool: shell
        command: pep8 *.py

Or, perhaps, supporting ansible snipets directly in the config? Something like this?

/pull-request:
    prepare:
        tool: ansible
        dnf:
            name: python3-pep8
            state: present
    execute:
        tool: shell
        command: pep8 *.py

Just brainstorming... But this does not look like super simple any more ;-)

@psss :) I believe this is a simple shell command really, what about

/pull-request:
    prepare:
        tool: shell
        command: dnf -y install python3-pep8
    execute:
        tool: shell
        command: pep8 *.py

I am also thinking about, if we should not also support just plain commands like:

/pull-request:
    execute:
        - dnf -y install python3-pep8
        - pep8 *.py

I'd be perfectly fine with making "to continue, we need these packages" as simple as possible, e.g.

/pull-request:
    requires:
        - python3-pep8
        # Or maybe better, given the different distros/environments the test may run on:
        - /usr/bin/pep8
    execute:
        tool: shell
        command: pep8 *.py

It'd be a step aside, from the "prepare - execute" flow, split into their separate sections, but on the other hand, a test scenario requiring additional packages is and will be one of the most common use cases. Definitely not a playbook for this kind of action, as I said, this is an extremely common use case that we shoudl simplify it:

• your TS needs some packages? List them under requires and you're done;
• your TS needs a bit more complicated setup? See docs for prepare (and that's for another discussion :)

I am also thinking about, if we should not also support just plain commands like:
/pull-request:
- dnf -y install python3-pep8
- pep8 *.py

I don't like the idea of putting them right under the TS, that will make implementation of the CI side extremely messy (our test-batch-dispatcher allows this and when one wants to add some flags, it just explodes...)

Sounds good. Maybe I would still keep the prepare section there:

/pull-request:
    prepare:
        requires: /usr/bin/pep8
    execute:
        tool: shell
        command: pep8 *.py

Just to make the high-level test execution steps separate.

@happz, added execute :) I am just thinking that we should make the shell execution somehow default, would make users happy

@happz, added execute :) I am just thinking that we should make the shell execution somehow default, would make users happy

That's very arguable, some users would be happy, I wouldn't (since I live in a wow/beakerlib/restraint world :)

Yeah, I think for each step there are (and will be) multiple ways how to do it. And I think we should support this. But make these different implementations distinct. Which leads me to another proposal: What about using how instead of tool? ;-)

That's very arguable, some users would be happy, I wouldn't (since I live in a wow/beakerlib/restraint world :)

right, I meant if somebody would would directly add a list under execute, it would be shell commands :)

in your case you would use tool: ... etc. variables to specify your flow :)

Yeah, I think for each step there are (and will be) multiple ways how to do it. And I think we should support this. But make these different implementations distinct. Which leads me to another proposal: What about using how instead of tool? ;-)

how sounds interesting to me :) I like it more as tool :)

I think we have to start with the structure before we start introducing shortcuts :) Let's get together, draft these parts and the specification - knowing what things we have to describe to make it all work together - and after that, we can think about making the most common cases simpler.

Plus one: Main structure first, then shortcuts and simplifications.

I think we have to start with the structure before we start introducing shortcuts :) Let's get together, draft these parts and the specification - knowing what things we have to describe to make it all work together - and after that, we can think about making the most common cases simpler.

Sounds reasonable, unfortunately this week I am WFH whole week :( so next week at earliest for me, or we can do BjN

Yeah, I think for each step there are (and will be) multiple ways how to do it. And I think we should support this. But make these different implementations distinct. Which leads me to another proposal: What about using how instead of tool? ;-)

Yep, sounds good to me :) Suits the question better: "how am I supposed to run the tests?"

Good, then let's do it: https://pagure.io/fedora-ci/metadata/pull-request/7

Updated according to the latest feedback on yesterday's meeting. Going to merge.