#240 Support for a simple fmf filter in beakerlib role
Merged by astepano. Opened by psss.
psss/standard-test-roles filter  into  master

Download 240.patch

This adds basic support for test selection based on metadata. The
new "filter" variable and dict key under "repositories" accepts a
single fmf filter. Separate script "str-filter-tests" is used to
check provided paths and filters and lists appropriate tests on
the standard output. Manual test listing can be combined with the
filter functionality as well.

Basic set of metadata added to shell tests for easier testing:
https://src.fedoraproject.org/tests/shell/c/64a77bb

Hi,
Do we need this a new script str-filter-tests files? Can we call fmf directly ?

I was thinking about this for a bit and I believe having a separate python script is the better way as it will allow us to more easily extend the functionality in the future. Doing this directly in ansible is quite cumbersome. It also allows to catch exceptions like fmf.utils.RootError properly.

rebased onto 79da1f9e717d4ec5f48531b545928cdbc68a3b4f

rebased onto de77e62e251916a6b6526c663699cfe6804787bb

Python code style errors fixed.

pretty please pagure-ci rebuild

Code style check seems to be fine now.
Could you please review the PR? Thanks.

Can we move this line before with_items? It is a bit confusing to read. But, again, it is OK.

Hi, what would be if both above examples put together ? Like:

   roles:
   - role: standard-test-beakerlib
     tags:
     - classic
     filter: "tier: 1"
     repositories:
     - repo: "https://src.fedoraproject.org/tests/shell.git"
       dest: "shell"
       filter: "tier: 1"

I think there will be collision. of filters for repo and tests. You cannot correctly construct filter_command

Hi, sorry for delayed answer.
I have a few proposals, just let us discuss:

  • filter keyword. How about change to tests_filter ? Lets take an example you provide:
     repositories:
     - repo: "https://src.fedoraproject.org/tests/shell.git"
       dest: "shell"
       filter: "tier: 1"
    

As you see we use filter keyword under repositories. It is not clear what is filter for repositories.

or:

   - role: standard-test-beakerlib
     tags:
     - classic
     filter: "tier: 1"

It is not clear what is filter for role.

Could we introduce some more clear this step? Some of:
1. Simple: add a comment what is a case when filter_command != "str-filter-tests"
2. Or better: add a filter_type, and check it with when: filter_type == git_repos ...

yeah, there will be definitely collision if filter is defined for role and for repo.

Maybe put update doc and say, only in one place it can be used? Or add two different filters "filter_gitrepos" and "filter_tests" ?

Otherwise, looks good.

Thanks for the review, Andrei. The implementation is done in a way that is should support all combinations: Filter in role, filter in repository and both. The filter command is extended for each appearing filter. That's why there's check for filter_command != "str-filter-tests" which means there is no filter defined. I don't see a reason why we should introduce filter_type there.

Regarding the parameter name: I see the short filter parameter could be a bit confusing, especially under the repositories. I'm OK with renaming that to test_filter or fmf_filter if you think it would be better.

1 new commit added

  • Incorporate pull request feedback

Feedback addressed:

  • Rename parameter to "fmf_filter" to make it more distinct
  • Extend comment for the str-filter-tests command
  • Gather results from filtered tests as well
  • Add forgotten link to fmf documentation

I've also fixed one more bug I ran into while testing the changes.

Commit 829ba507 fixes this pull-request

Pull-Request has been merged by astepano

Pull-Request has been merged by astepano

Metadata