Adds new function for empty env files #3204
Conversation
tmt/base.py
Outdated
|
|
||
| env_file = self.plan_environment_file | ||
|
|
||
| if (self.my_run and env_file.exists() and env_file.stat().st_size == 0): |
There was a problem hiding this comment.
I'm not sure this would work: by design, lint_* callbacks are called not in runtime, tmt run ..., but e.g. before committing changes, via tmt * lint. Therefore self.my_run should always be unset when linters run.
I think there is also a misunderstanding in your way: plan_environment_file is a per-plan file, provided by tmt to plans and tests, with path exposed via TMT_PLAN_ENVIRONMENT_FILE envvar, and as such does exist only in the context of tmt run. Which makes it non-lintable entity.
If you wanted to check whether files referenced by plan's environment-file, https://tmt.readthedocs.io/en/stable/spec/plans.html#environment-file, you'd need to check self.node, self.node.get("environment-file") or [] should give you the list of files referenced by the plan.
There was a problem hiding this comment.
@happz thanks for the feedback I am still a newbie here, just a couple of comments. The idea of this patch is to cover the issue #2872 which mentions to report the fail in the tmt lint so that should be the idea right?, or maybe you want to rethink the intention of the issue. Regarding the plan environment file I changed the code according to your review, I hope now it's appropiate
cc @psss
There was a problem hiding this comment.
I think the idea matches the issue, you just focused on the wrong "environment file", that's all :)
mcasquer
force-pushed
the
2872_empty_env_files
branch
from
6819c26
to
e1db5ce
Compare
tmt/base.py
Outdated
| """ P001: env files are not empty """ | ||
|
|
||
| env_files = self.node.get("environment-file") or [] | ||
| if len(env_files) > 0: |
There was a problem hiding this comment.
if env_files: is the Python way of checking list emptiness.
tmt/base.py
Outdated
| if len(env_files) > 0: | ||
| for env_file in env_files: | ||
| path = os.path.normpath(env_file) | ||
| size = os.path.getsize(path) |
There was a problem hiding this comment.
Use pathlib instead of os.path, please. It will eventually make the condition simpler as well:
for _env_file in env_files:
env_file = Path(env_file).resolve()
if not env_file.exists() or not env_file.stat().st_size:
..Catches both cases when the file does not exists - deserves its own message - and the file being empty.
mcasquer
force-pushed
the
2872_empty_env_files
branch
from
e1db5ce
to
ef711a8
Compare
New function that raises a failure if the size of the environment file, if this one exists, is zero. Signed-off-by: mcasquer <[email protected]>
mcasquer
force-pushed
the
2872_empty_env_files
branch
from
ef711a8
to
c2b295a
Compare
New function that raises a failure if the size of
the environment file, if this one exists, is zero.
Pull Request Checklist