Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tmt t lint should try to cover adjust rules as well #898

Open
lukaszachy opened this issue Sep 22, 2021 · 4 comments · May be fixed by #1334
Open

tmt t lint should try to cover adjust rules as well #898

lukaszachy opened this issue Sep 22, 2021 · 4 comments · May be fixed by #1334
Assignees
Labels
command | lint tmt lint command

Comments

@lukaszachy
Copy link
Collaborator

I made typo in adjust rules, tmt tests lint didn't show any error: (enable vs enabled)
Since adjust keys are defined (when, because and every tmt attribute for its object) we should be able to lint for unknown/typos

test: foo
adjust:
- when: distro == fedora
  enable: false
@Athwale
Copy link

Athwale commented Oct 1, 2021

Also when you forget the "when" keyword, lint crashes and you get a long traceback.
The last line is fmf.utils.FormatError: No condition defined in adjust rule. That already looks like something that could be nicely printed as an error.

@idorax idorax self-assigned this Jul 11, 2022
@idorax
Copy link
Contributor

idorax commented Jul 11, 2022

For adjust part,

test: foo
adjust:
- when: distro == fedora
  enable: false

self.node.get().keys() in tmt/base.py#L292 does return the L1 keys and doesn't return the L2 keys(e.g. when and enable), that's why tmt t lint failed to detect key enable is invalid (p.s. it should be enabled).

(Pdb) b tmt/base.py:291
Breakpoint 1 at /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/base.py:291
(Pdb) c
/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
> /home/huanli/.virtualenvs/tmt/lib/python3.10/site-packages/tmt/base.py(291)lint_keys()
-> known_keys = additional_keys + self._keys
(Pdb) ll
289  	    def lint_keys(self, additional_keys):
290  	        """ Return list of invalid keys used, empty when all good """
291 B->	        known_keys = additional_keys + self._keys
292  	        return [key for key in self.node.get().keys() if key not in known_keys]
(Pdb) pp additional_keys, self._keys
(['extra-nitrate',
  'extra-hardware',
  'extra-pepa',
  'extra-summary',
  'extra-task',
  'relevancy',
  'coverage',
  'adjust'],
 ['summary',
  'description',
  'contact',
  'component',
  'test',
  'path',
  'framework',
  'manual',
  'require',
  'recommend',
  'environment',
  'duration',
  'enabled',
  'order',
  'result',
  'tag',
  'tier',
  'link'])
(Pdb) pp self.node.get()
{'adjust': [{'enable': False, 'when': 'distro == fedora'}], 'test': 'foo'}
(Pdb) pp self.node.get().keys()
dict_keys(['adjust', 'test'])
# ^^^^^^^^^^^^^^^ It doesn't include key 'enable' and 'when' !!!

Hence, it looks we should get the L2 keys at tmt/base.py#L292.

@idorax
Copy link
Contributor

idorax commented Jul 11, 2022

To get L2 keys in property adjust, we can iterate self.node.get('adjust'), e.g.

(Pdb) p self.node.get()
{'adjust': [{'when': 'distro == fedora', 'enable': False}], 'test': 'foo'}
(Pdb) prop_adjust = self.node.get('adjust')
(Pdb) adjust_keys = [key for item in prop_adjust for key in item.keys()]
(Pdb) pp adjust_keys
['when', 'enable']

idorax pushed a commit to idorax/tmt that referenced this issue Jul 11, 2022
idorax pushed a commit to idorax/tmt that referenced this issue Jul 11, 2022
@idorax idorax linked a pull request Jul 11, 2022 that will close this issue
@idorax
Copy link
Contributor

idorax commented Mar 8, 2024

Here is a brief summary after discussing with TMT developers.

  • FMF supports some suffixes, e.g. +, -, +<. But currently we just check for the validity of the key names and suffixes supported by FMF for the time being.
  • In the future we can do a cleaner solution in FMF, e.g. use regular expressions for the supported suffixes.

@psss psss removed the enhancement label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command | lint tmt lint command
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants