-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add support for static linters #200
Conversation
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
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 think that --linters and --static linters could be used together. --static linters
is just subset, so in case both are typer, it is just enough to ignore this --static linters
. But it can be solved in this way.
from moduleframework import module_framework | ||
from moduleframework import dockerlinter | ||
|
||
class DockerInstructionsTests(module_framework.AvocadoTest): |
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.
you've forgot to change module_framework.AvocadoTest
to Test
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 use assert_to_warn
function. AvocadoTest is needed. I don't see any reason, why not use AvocadoTest.
if self.dp.dockerfile is None: | ||
self.skip("Dockerfile was not found") | ||
|
||
def tearDown(self, *args, **kwargs): |
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.
when you change parent to Test
you can remove this empty tearDown
function
|
||
import os | ||
from avocado import Test | ||
from moduleframework import module_framework |
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.
probably this import could be removed
moduleframework/common.py
Outdated
@@ -91,6 +91,7 @@ | |||
MODULE_DEFAULT_PROFILE = "default" | |||
TRUE_VALUES_DICT = ['yes', 'YES', 'yes', 'True', 'true', 'ok', 'OK'] | |||
OPENSHIFT_INIT_WAIT = 50 | |||
STATIC_CHECKS = ['static_dockerfiule_checks', 'helpmd_lint'] |
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.
wouldn't be better to add there some same pattern?
like you have: static_docker...
and so that remane also helpmd_lint to same pattern.
otherwise there is typo ...fiule...
…atic Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
I tried this in container environment and everything runs as expected. There is still need to provide URL, but that issue is not blocking now and can be resolved in #84. Thank you! :) |
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.
Looks good. just few small comments.
moduleframework/mtf_scheduler.py
Outdated
if args.linter: | ||
self.tests += glob.glob("{MTF_TOOLS}/*.py".format( | ||
MTF_TOOLS=metadata_common.MetadataLoaderMTF.MTF_LINTER_PATH)) | ||
self.tests += glob.glob("{MTF_TOOLS}/{GENERIC_TEST}/*.py".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.
this change cause regression (now args.linter schedule all generic tests) with your change, just part of them will be sheduled. Is that intentional? we can discuss it. I'm not against this idea, but we should think about it more.
|
||
import os | ||
from avocado import Test | ||
from moduleframework import module_framework |
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.
remove this line and remove module_framework.AvocadoTest
from class parent
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.
It is not possible, because we have in AvocadoTest
function assert_to_warn
.
from moduleframework import module_framework | ||
from moduleframework import dockerlinter | ||
|
||
class DockerInstructionsTests(module_framework.AvocadoTest): |
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.
replace by inherited from Test
Signed-off-by: Petr "Stone" Hracek <phracek@redhat.com>
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.
LGTM
Signed-off-by: Petr "Stone" Hracek phracek@redhat.com
This Pull Request brings new argument into
mtf
command like --static-linterswhich executes only helpmd_linter.py and dockerlint.py -> static-dockerlint-checks.py.