RedHatInsights / insights-client

insights-client
Other
27 stars 49 forks source link

test: add insights-client test case for tags #293

Closed zhangqianqian closed 1 month ago

zhangqianqian commented 2 months ago

This PR test how the tags generated, and check the tags on inventory.

zhangqianqian commented 2 months ago

Generally LGTM from my point of view. Also, there's a typo in the title, clilent.

Thanks, has modified the PR title. @m-horky would you pls help merge it?

zhangqianqian commented 2 months ago

@m-horky modified the PR again as your suggestions. pls merge it.

zhangqianqian commented 1 month ago

Thanks for the test! It seems to work fine, there are few small things to fix.

Since this is a new test, please write a proper docstring for it following the format of Testimony [1]; see also the internal CCT-657 and all the recent PRs done by Zdenek on the topic.

[1] https://testimony-qe.readthedocs.io/en/stable/

There is a lot of repetition of the path /etc/insights-client/tags.yaml all around the test, and lots of manual I/O: let's simplify it using pathlib.Path; declate it as global variable in constants.py:

import pathlib

[...]
TAGS_FILE = pathlib.Path("/etc/insights-client/tags.yaml")

Then it can be used easily -- few examples:

    with contextlib.suppress(FileNotFoundError):
        TAGS_FILE.unlink()
[...]
        assert not TAGS_FILE.exists()
[...]
    with TAGS_FILE.open("r") as tags_yaml:

Do not forget to remove the tags file at the end of the test; to make sure to remove it even in case the test fails, you can use contextlib.ExitStack():

    with contextlib.ExitStack() as stack:
        # Run insights-client --group
        insights_client.run("--group=first_tag")
        stack.callback(os.remove, TAGS_FILE)

        [the rest of the test until the end]

Thanks Pino.