Conversation
a5c8249 to
f8c4ef7
Compare
TomasTomecek
left a comment
There was a problem hiding this comment.
It would be for the best to discuss this in person.
colin/checks/dockerfile.py.fmf
Outdated
| @@ -0,0 +1,19 @@ | |||
| description: "asdasdsfdasfsda f" | |||
There was a problem hiding this comment.
what format is this? is it yaml?
colin/core/checks/abstract_check.py
Outdated
| def _get_fmf_metadata(self): | ||
| output = {} | ||
| classfile = inspect.getfile(self.__class__) | ||
| fmf_tree = fmf.Tree(os.path.dirname(classfile)) |
There was a problem hiding this comment.
does this mean that we would have to store the metadata alongside the .py file?
colin/core/checks/abstract_check.py
Outdated
| setattr(self, k, metadata_dict[k]) | ||
|
|
||
| def __init__(self, message=None, description=None, reference_url=None, tags=None): | ||
| if message or description or reference_url or tags: |
There was a problem hiding this comment.
better logic could be to load the metadata and just enable overriding them
af8f0e9 to
caf053c
Compare
| @@ -0,0 +1,188 @@ | |||
| #!/usr/bin/python3 | |||
There was a problem hiding this comment.
Since this script actually proposes a new runtime, I would personally prefer to have it proposed in a different PR. I actually dislike that it's not integrated into colin's CLI interface.
There was a problem hiding this comment.
yep, this is my suggested solution, split this PR, to 2 or 3 PRs, and last of them will be this nosetests generator.
But on other hand, without this, it is harder to show benefits eg:
- write tests just in metadata without empty python classes
- using nosetests scheduled tests
- filtering and selecting or test items
- using FMF rulesets
And this PR will just split metadata outside python classes to FMF files.
But I agree and with this knowledge, that these PRs are closely connected I can split it.
| reference_url="https://fedoraproject.org/wiki/Container:Guidelines#FROM", | ||
| tags=["from", "dockerfile", "baseimage", "latest"]) | ||
| class FromTagNotLatestCheck(FMFAbstractCheck, DockerfileAbstractCheck): | ||
| name, metadata = FMFAbstractCheck.get_metadata("from_tag_not_latest") |
There was a problem hiding this comment.
I wonder whether there is a nicer way of doing this. Calling functions on class' attributes outside of methods seems sketchy.
There was a problem hiding this comment.
Yep, I had also troubles with this, but actually there is probably not better way how to do it without bigger changes in colin. Because current colin implementation does static inspection of classes, to find checks based on class attribute name. If you have some idea I'll be happy to change it to better way
FMF Metadata works well
FMF metadata support in colin. There are two different parts
TODOs:
research how to ideally solve two
__init__()or how to check that all variables are setfind way how to name tests
transform all
__init__s to FMF compatible wayfind way how transform rulesets to FMF testsets
remove all duplicated stuff from colin what is already implemented by FMF** (next PRs)
Colin output:
$ python -m colin.cli.colin list-checkspython -m colin.cli.colin check tests/data/DockerfileFMF output
fmf show --path colin/checkswhen I want to be supercool and have backward compilant output:
use format abilites of fmf:
fmf show --path colin/checks --format "{}\n -> {}\n -> {}\n -> {}\n -> {}\n\n" --value name --value "data['description']" --value "data['message']" --value "data['reference_url']" --value "data['tags']"Nosetests output - direct
TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.pyusing rulesets
advanced filtering
possible to apply them to rulesets or directy to testcases
FILTERS=tags:dockerfile TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py$ NAMES="from_tag_not_latest;maintainer_label" TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.pyrulesets filters
$ NAMES=default RULESETPATH=rulesets/ TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py$ NAMES=fedora RULESETPATH=rulesets/ TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.pyfedora ruleset is not given there, so that it runs 0 tests