Add a `lint` check for empty and missing environment files

[mcasquer]

New function that raises a failure if the size of the environment file, if this one exists, is zero.

Pull Request Checklist

• [x] implement the feature
• [x] extend the test coverage
• [x] include a release note

[psss]

Would you mind adding a simple test coverage? You could just extend the existing test. Also, what about adding a short release note?

@mcasquer, what do you think about adding a simple test and a short release note?

[mcasquer]

Would you mind adding a simple test coverage? You could just extend the existing test. Also, what about adding a short release note?

@mcasquer, what do you think about adding a simple test and a short release note?

Perfect, I will add them along with the above suggestions !

[psss]

/packit test

[psss]

/packit build

[psss]

Test /tests/lint/plan/explicit-root is failing. Seems that your newly added test coverage has revealing a bug. Congratulations! :) Here are simple steps to reproduce:

> cd tests/lint/plan/data
> tmt plan lint good
/good
pass C000 fmf node passes schema validation
pass C001 summary key is set and is reasonably long
pass P001 correct keys are used
pass P002 execute step defined with "how"
pass P003 execute step methods are all known
skip P004 discover step is not defined
skip P005 no remote fmf ids defined
pass P006 phases have unique names
pass P007 execute phase 'default-0' does not require specific guest
pass P008 no empty environment files

Lint checks on all
pass G001 no duplicate ids detected

> cd ..
> tmt --root data plan lint good

[Errno 2] No such file or directory: '/home/psss/git/tmt/tests/lint/plan/env'

Both commands should give the same result as the --root option is instructing tmt to use provided directory as the fmf metadata tree root. Seems that path to the env file is not correctly constructed when --root is used. @mcasquer, could you please have a look?

[mcasquer]

Both commands should give the same result as the --root option is instructing tmt to use provided directory as the fmf metadata tree root. Seems that path to the env file is not correctly constructed when --root is used. @mcasquer, could you please have a look?

@psss Ok I will take a look :)

[mcasquer]

Both commands should give the same result as the --root option is instructing tmt to use provided directory as the fmf metadata tree root. Seems that path to the env file is not correctly constructed when --root is used. @mcasquer, could you please have a look?

@psss Is this the only code that implements this --root feature? https://github.com/teemtee/tmt/blob/b058850af4947c4dc2152a428d5f04a66dc1cfaa/tmt/cli.py#L290-L292

[happz]

@mcasquer correct. Once an fmf tree is loaded, objects derived from it - tests, plans, stories - should have fmf_root attribute storing the path. And I'd say that's what's missing: env_file = Path(env_file).resolve() should be env_file = (self.fmf_root / Path(env_file)).resolve() - or, better, once https://github.com/teemtee/tmt/pull/3212 gets merged, env_file = (self.anchor_path / Path(env_file)).resolve().

[mcasquer]

@mcasquer correct. Once an fmf tree is loaded, objects derived from it - tests, plans, stories - should have fmf_root attribute storing the path. And I'd say that's what's missing: env_file = Path(env_file).resolve() should be env_file = (self.fmf_root / Path(env_file)).resolve() - or, better, once #3212 gets merged, env_file = (self.anchor_path / Path(env_file)).resolve().

@happz that's indeed the point, thanks a bunch! I will change it then to the final version (once it's merged) as I see at this moment the #3212 is planned for 1.38 milestone too

[mcasquer]

@psss @happz everything should be fine now :grinning:

[psss]

@psss @happz everything should be fine now 😀

Great thanks! In 10177f3b I've just added back the minor adjustments which were lost in the rebase.

[psss]

/packit test

[psss]

/packit build

[mcasquer]

@psss it seems the data directory is not included in the path when doing the lint Test result

fail P008 the environment file '/var/ARTIFACTS/work-coreu1ik6621/plans/features/core/discover/default-0/tests/tests/lint/plan/data/empty_env' is empty`

vs what the test expects

:: [ 10:33:43 ] :: [   FAIL   ] :: File '/var/tmp/rlRun_LOG.rrmwXJFk' should contain 'fail P008 the environment file '/var/ARTIFACTS/work-coreu1ik6621/plans/features/core/discover/default-0/tests/tests/lint/plan/empty_env' is empty' 

[psss]

Yeah, the problem was in the fact, that the test is run twice, each time in a different directory:

https://github.com/teemtee/tmt/blob/7c08e96170470eb6e3cb99fbf8b705862a1e0a58/tests/lint/plan/test.sh#L6-L11

So we cannot use $(pwd) there. I think we're good with a basic check. What about fba14f24?

[mcasquer]

So we cannot use $(pwd) there. I think we're good with a basic check. What about fba14f2?

Ok I see, looks fine for me now, thanks ! :)

[psss]

Everything green, the epel-9 job actually finished, looks just like some packit-github sync issue, merging.