-
Notifications
You must be signed in to change notification settings - Fork 27
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
unittest generator and scheduler based on FMF metadata #161
base: master
Are you sure you want to change the base?
Conversation
colin/core/checks/fmf_check.py
Outdated
""" | ||
reference_nodes = self.prune(whole=whole, names=["@"]) | ||
for node in reference_nodes: | ||
node.data = node.original_data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
node.data is not used in function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is intentional, this allows us to modify FMF to allow using rulesets with full power of FMF. in this case +
sign for nodes.
Basically, FMF removes python code into fmf (YAML) format. I don't see any advantage for colin alone. I would like to see the other projects which will use FMF. I see only colin uses FMF, like PoC for using FMF, what about to wait till the other project will use/implement it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix my issues.
colin/core/checks/fmf_check.py
Outdated
logger.debug("get FMF metadata for test (path:%s name=%s)", fmfpath, name) | ||
items = [x for x in fmf_tree.climb( | ||
) if name in x.name and "@" not in x.name] | ||
if object_list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if bool is set return all items? Why, I don't understand this part of code.
colin/core/checks/fmf_check.py
Outdated
if len(items) == 1: | ||
output = items[0] | ||
elif len(items) > 1: | ||
raise Exception("There is more FMF test metadata for item by name:{}({}) {}".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
naming would be nice like n=name, l=len(items).
colin/core/checks/fmf_check.py
Outdated
@classmethod | ||
def get_metadata(cls, name, path=None): | ||
""" | ||
Very important method what returns tuple for object initialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get rid of documentation like 'Very important method'. It is enough to say. 'Method returns ..'
fmf_scheduler.py
Outdated
return test | ||
|
||
|
||
def make_base_fmf_class_abstract(node, target, base_class=DynamicClassBase, fmf_class=FMFAbstractCheck): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring is missing.
fmf_scheduler.py
Outdated
return outclass | ||
|
||
|
||
def make_wrapped_fmf_class(node, target): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function is a bit complicated. What about to add an example into docstring for better understanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add, good idea. whole idea of this class is to create wrapper class like (in short):
class testClass(unittest):
backendclass = abstract_check_inherited_class
def test():
return backendclass().check(TARGET)
is this sufficient comment in doc string
fmf_scheduler.py
Outdated
return test_classes | ||
|
||
|
||
def scheduler_opts(target_name=None, checks=None, ruleset_path=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
colin functionality has to remain as it is. scheduler_opts has to be valid only for FMF.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, thats why it is directly in FMF scheduler and not inside colin, but finally we, remove colin and use this and make it backward complatible, so finally there will be no changes for current colin users
I am afraid of using FMF in colin. |
unittest dynamic class generator
you can use this class file as dynamic generator for another scheduler like
nosetest
scheduler possibilities
usage like: