tmt tests lint --fix does not process virtual cases correctly

[sopos]

tmt tests lint does not complain about the relevancy and --fix does not fix that in case of a virtual case.

E.g. the test https://github.com/RedHat-SP-Security/tests/tree/master/rsyslog/Regression/bz1223566-RFE-imrelp-does-not-support-binding-to-a-specific

[lukaszachy]

Thanks, simplified reproducer:

==> a.fmf <==
tag: []

==> main.fmf <==
test: echo
relevancy: |
  distro == rhel: False

I'm lazy to file another issue as it can be connected, but if b.fmf is empty file then lint fails with

$ touch b.fmf
$ tmt t lint

  File "/usr/lib/python3.10/site-packages/tmt/cli.py", line 418, in lint_tests
    if not test.lint():
  File "/usr/lib/python3.10/site-packages/tmt/base.py", line 488, in lint
    relevancy = metadata.pop('relevancy', None)
AttributeError: 'NoneType' object has no attribute 'pop'

[lukaszachy]

Looks tmt lint should try to find where 'relevancy' was used and change that file, not final metadata - https://github.com/teemtee/tmt/blob/main/tmt/base.py#L504

[idorax]

If an empty file is specified, tmt t lint looks good now because of #1332.

(tmt) huanli@kvm1:foo$ tree -a
.
├── 1197
│   └── b.fmf
├── .fmf
│   └── version
└── main.fmf

2 directories, 3 files
(tmt) huanli@kvm1:foo$ cat main.fmf
test: echo
relevancy: |
  distro == rhel: False
(tmt) huanli@kvm1:foo$ cat 1197/b.fmf
(tmt) huanli@kvm1:foo$ tmt tests ls
/1197/b
(tmt) huanli@kvm1:foo$ tmt t lint
/1197/b
pass test script must be defined
pass directory path must be absolute
pass directory path must exist
warn summary is very useful for quick inspection
pass correct attributes are used

[idorax]

@lukaszachy, with tmt-1.15.0, tmt t lint passed. Should tmt t lint complain relevancy is wrong? If yes, what kind of error should it complain?

(tmt) huanli@kvm1:foo$ tree -a
.
├── 1197
│   └── a.fmf
├── .fmf
│   └── version
└── main.fmf

2 directories, 3 files
(tmt) huanli@kvm1:foo$ cat main.fmf 
test: echo
relevancy: |
  distro == rhel: False
(tmt) huanli@kvm1:foo$ 
(tmt) huanli@kvm1:foo$ cat 1197/a.fmf
tag: []
(tmt) huanli@kvm1:foo$ 
(tmt) huanli@kvm1:foo$ tmt tests ls
/1197/a
(tmt) huanli@kvm1:foo$ tmt t lint
/1197/a
pass test script must be defined
pass directory path must be absolute
pass directory path must exist
warn summary is very useful for quick inspection
pass correct attributes are used

[lukaszachy]

@idorax If you remove a.fmf from the repruducer then tmt t lint does 'fail', which should be the outcome also when 'a.fmf' is present.

There seems to be few additional ways how to smuggle 'relavancy' into the test and are not linted properly right now:

• relevancy added in virtual test defined in main.fmf, e.g.

$ cat main.fmf
test: echo
/foo:
  relevancy: |
    distro == rhel: False

• main.fmf few levels above (current reproducer is for main.fmf in the same directory)
• really a nitpick but relevancy added in adjust rule (imho do not invest time here unless it is side effect of real fix).

To make things worse - If you operate on 'raw' data you might have 'relevancy+' key as well --fix should remove that too.
There might be cases when --fix might affect other tests (e.g. tmt t lint --fix b for bellow you can't remove from 'main.fmf' without affecting '/a' test)

==> a.fmf <==
relevancy+: |
  distro == fedora: False

==> b.fmf <==
relevancy+: |
  distro >= fedora-38: False

==> main.fmf <==
test: echo
relevancy: |
  distro == rhel: False