feat(integration tests): Utilities for testing ansible-trace #15
Conversation
feat(action): Change python version in action CI feat(action): Rename file CI fix(action): remove useless file
| @@ -0,0 +1,11 @@ | |||
| [all] | |||
| 127.0.0.1 ansible_connection=local | |||
There was a problem hiding this comment.
This is pretty clever, using multiple localhosts. Nice idea
| @@ -0,0 +1,3 @@ | |||
| import pytest | |||
| from fixtures import ansible_play | |||
|
|
|||
There was a problem hiding this comment.
What's this file? Probably missing something but is it empty? Shouldn't there be, like, a test function or something in here? (I'm not very familiar with pytest)
There was a problem hiding this comment.
It is to import the fixture on every other pytest test file
tests/integration/tests/fixtures.py
Outdated
|
|
||
| # Set params for ansible runner | ||
| request.config.option.ansible_playbook_directory = "." | ||
| request.config.option.ansible_playbook_inventory = "inventory.ini" |
There was a problem hiding this comment.
Is there a reason we can't do = inventory ? Wouldn't that let us avoid the symlinking and renaming (and the resulting side-effects on the filesystem?)
There was a problem hiding this comment.
Oops my bad, forgot to unstash some of my work, will definitely fix that
tests/integration/tests/utils.py
Outdated
|
|
||
| return (hosts, duration_events, duration_stacks) | ||
|
|
||
| def add_duration_event(hosts: Dict[int, HostEvent], events: Dict[int, Any], curr_stacks: Dict[int, Deque], event_hash: Any) -> Tuple[Dict[int, Any], Dict[int, Deque]]: |
There was a problem hiding this comment.
Would you mind wrapping some of these very long lines? Whatever the other python files in the repo are wrapped at
tests/integration/tests/basic.py
Outdated
| trace_hosts: Dict[int, HostEvent] | ||
| trace_events: Dict[int, Any] | ||
| trace_json: JSONTYPE = get_last_trace() | ||
| trace_hosts, trace_events = parse_trace(trace_json) |
There was a problem hiding this comment.
This last line feels a little funny; we're assigning variables we aren't using. I had to read the parse_trace to figure out that the bulk of the test is in there: that it's validating the trace as it's parsing. Could we name the function to make clear it's validating too? parse_and_validate_trace or something?
There was a problem hiding this comment.
Yea I will !
|
This overall looks really good to me. Thanks so much for writing these tests. I really like how they're integration-level tests that actually run Ansible, and have semantic validations on the output (e.g. timestamps nest). |
|
Review taken in account ! Lint pep8, removed the symlinks stuff, and changed sub name. Do not hesitate if something is still not good! |
Hello everyone 👋 !
This is the first step for #10
This proposition allow creating tests using pytest framework where we can chose strategy, inventory for the same playbook file (see tests/integration/tests/basic.py)
I have also made some utilities functions that parse trace to ensure its integrity.
This include also related github action CI.yml (here a sample)
Waiting for your feedback,
Thanks!