#139 WIP: adding tests to be run with playbook
Closed by jheger. Opened by jheger.
jheger/standard-test-roles feature/test-filtering  into  master

Download 139.patch

JIRA issue: https://projects.engineering.redhat.com/browse/RHELPLAN-3428

This commit adds new variable to standard-role-beakerlib tests_generator_cmd which should contain bash command producing absolute paths to tests (one path per line) you wish to add to list of tests being run with the playbook.

Usage:
tests_generator_cmd can be either written into playbook as variable:

---
# Tests that run in classic context
- hosts: localhost
  roles:
  - role: standard-test-beakerlib
    tags:
    - classic
    tests:
    - func
    - login-shell
    - smoke
    - substitution-in-bash
    required_packages:
    - expect            # login-shell requires expect command
    - findutils         # beakerlib needs find command
    - which             # smoke requires which command
    tests_generator_cmd: 'fmf --brief --key test --format "{}/{}\n" --value "root" --value "data.get(''path'') or name"'

Pull request also changes task which copies tests to working directory (name: Copy tests to test environment), as I found the original way was redundant, I would appreciate feedback on this as well the whole pull request.

Task/variable names are subject to change.

@jheger first of all I would like to see the name of the option to be different and reflect this is extending the tests list. Some suggestions from me:

tests_command: 'CMD'
tests_generetor_command: 'CMD'

please use the synchronize module, the copy is a lot slower :(

this is required for the synchronize module

Hmm, we are now copying only a subset of tests and excluding tests.yml, not sure if that is expected .. needs testing

@jheger, hmm so you careate tests_list but later on use only tests variable to run them? That seems odd ...

https://pagure.io/fork/jheger/standard-test-roles/blob/feature/test-filtering/f/roles/standard-test-beakerlib/tasks/main.yml#_114

?

@jheger, hmm so you careate tests_list but later on use only tests variable to run them? That seems odd ...
https://pagure.io/fork/jheger/standard-test-roles/blob/feature/test-filtering/f/roles/standard-test-beakerlib/tasks/main.yml#_114
?

This is a mistake I forgot to change those variables

1 new commit added

  • Forgot to change variables later in the role

This command will be run on 'test-environment'.

  ls -d ".... full path"

Question is: "Does test-environment system have tests at this point?"

@jheger What is the reason to replace synchronize with copy?

@jheger What is the reason to replace synchronize with copy?

No particular reason, I saw it being used earlier in the role so I used it as well. I can change it to synchronize.

2 new commits added

  • changed module of test copying task
  • changed var name generate_cmd -> tests_generator_cmd

@jheger What is the reason to replace synchronize with copy?

No particular reason, I saw it being used earlier in the role so I used it as well. I can change it to synchronize.

Please do this. PR should have logical changes, that are related to the topic of of PR. If you introduce some change than I assume that it is somehow connected to PR.

Hmm, we are now copying only a subset of tests and excluding tests.yml, not sure if that is expected .. needs testing

@mvadkert actually synchronize src: {{ playbook_dir }} dest: {{ tenv_workdir }} is done before beakerlib-role even starts from role str-common, which is included in beakerlib-role.
So I thought it is enough to copy only the tests to the {{ tenv_workdir }}.

pretty please pagure-ci rebuild

pretty please pagure-ci rebuild

Shall we use something shorter then tests_generator_cmd?
Perhaps tests_command would be enough?

We've discussed this with Miro a bit more and we agreed that the complex syntax using --format and --value is not something that a regular user would be able to quickly understand and easily remember and use. What about using the last example from the wiki?

https://fedoraproject.org/wiki/CI/Metadata/Examples#Integration

We could call the variables fmf_directories or fmf_trees for the list of directories to be inspected and fmf_filters for the list of filters to be applied. This could be a simple step forward without introducing too much complexity. What do you think?

Here's summary from yesterday's discussion with @jheger and @mvadkert: There should be a single filter variable which could used in the following two ways:

For each fetched repository under repositories variable:

repositories:
  - repo: "https://src.fedoraproject.org/tests/selinux.git"
    dest: "selinux"
    filter: "tags:Tier1"

Or a general filter for tests stored directly in dist-git:

  - role: standard-test-beakerlib
    tags:
    - classic
    filter: "tags:Tier1"
    ...

For now filter should support only a single value. In future we can consider supporting list of filters as well if needed.

Support for simple filtering implemented here:
https://pagure.io/standard-test-roles/pull-request/240
We can probably close this pull request now.

Pull-Request has been closed by jheger

Metadata