the role junit2json #591
the role junit2json #591
Conversation
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 51s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 44s |
d63179e to
65426e8
Compare
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 42s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 48s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 44s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 46s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 43s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 44s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 3m 05s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 42s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 3m 18s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 59s |
roles/junit2json/tasks/convert.yml
Outdated
| ansible.builtin.set_fact: | ||
| junit2json_xml_report_hash_path: "{{ xml_report }}.{{ junit2json_hash }}" | ||
| junit2json_xml_report_hash_curr: "{{ junit2json_xml_report_content | ansible.builtin.hash(junit2json_hash) }}" | ||
| junit2json_xml_report_hash_old: "" |
There was a problem hiding this comment.
Do we need it here if it's empty?
There was a problem hiding this comment.
why is this conversation resolved? I don't see an answer
| when: | ||
| - junit2json_xml_report_hash_curr != junit2json_xml_report_hash_old | ||
|
|
||
| - name: Write json data to destination |
There was a problem hiding this comment.
For the following two Write tasks, you can use a block with a single when conditional, as they share the same logic and content approx.
There was a problem hiding this comment.
Please reply before resolving.
roles/junit2json/tasks/main.yml
Outdated
| --- | ||
| # tasks file main.yml for role redhatci.ocp.junit2json | ||
|
|
||
| - name: Validate some variables |
There was a problem hiding this comment.
I think this is more readable:
- name: Validate required variables
ansible.builtin.assert:
that:
- global_reports_path_patterns | length > 0
- junit2json_input_reports_list | length > 0
- junit2json_output_dir | length > 0
fail_msg: "One or more required variables are missing or empty"
There was a problem hiding this comment.
If you agree with what I wrote, please change it.
roles/junit2json/tasks/main.yml
Outdated
| ansible.builtin.assert: | ||
| that: "{{ condition }}" | ||
| loop: | ||
| - global_reports_path_patterns | length > 0 |
There was a problem hiding this comment.
This variable should follow the naming convention of the role (be prefixed by junit2json_ keyword)
roles/junit2json/tasks/expand.yml
Outdated
| - name: Setup helper variable | ||
| ansible.builtin.set_fact: | ||
| junit2json_find_patterns: "{{ global_reports_path_patterns | map('regex_replace', '^', '*.') | list }}" | ||
| cacheable: false |
There was a problem hiding this comment.
Var to be prefixed by junit2json keyword
roles/junit2json/tasks/main.yml
Outdated
| file: expand.yml | ||
| loop: "{{ junit2json_input_reports_list }}" | ||
| loop_control: | ||
| loop_var: path_item |
There was a problem hiding this comment.
maybe this var could be renamed to _junit2json_path_item
roles/junit2json/tasks/merge.yml
Outdated
| - junit2json_debug | ||
|
|
||
| - name: Merge multiple JUnit XML files into single consolidated report | ||
| ansible.builtin.shell: |
There was a problem hiding this comment.
If just using one single command, please use command module instead of shell
roles/junit2json/vars/main.yml
Outdated
There was a problem hiding this comment.
if this file is commented, could it be removed or could you move this content to the README?
roles/junit2json/tasks/convert.yml
Outdated
|
|
||
| - name: Update global_json_reports_list | ||
| ansible.builtin.set_fact: | ||
| global_json_reports_list: "{{ global_json_reports_list | default([]) + [junit2json_output_report_path] }}" |
There was a problem hiding this comment.
you should prefix this var with junit2json if possible
There was a problem hiding this comment.
thank you, @ramperher ,
only role's variables should have role specific prefix (junit2json_ here)
global_ is a legal prefix for cross-role variables.
There was a problem hiding this comment.
Got it, thanks.
Taking that into account, where is this global variable intended to be used? Is it going to be consumed by another role from the collections?
There was a problem hiding this comment.
I also don't see an answer to this question, please answer before resolving the convos.
More feedback: you could put global_json_reports_list in your defaults file and you won't have to use default() here or pass [] as a variable
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 54s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 46s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 42s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 48s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 41s |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 3m 42s |
|
|
||
|
|
||
|
|
||
| - **junit2json_input_merged_report** |
There was a problem hiding this comment.
this should have a sane default
|
|
||
|
|
||
|
|
||
| - **junit2json_output_merged_report** |
There was a problem hiding this comment.
this should also have a sane default
roles/junit2json/tasks/convert.yml
Outdated
| ansible.builtin.set_fact: | ||
| junit2json_xml_report_hash_path: "{{ xml_report }}.{{ junit2json_hash }}" | ||
| junit2json_xml_report_hash_curr: "{{ junit2json_xml_report_content | ansible.builtin.hash(junit2json_hash) }}" | ||
| junit2json_xml_report_hash_old: "" |
There was a problem hiding this comment.
why is this conversation resolved? I don't see an answer
roles/junit2json/tasks/convert.yml
Outdated
|
|
||
| - name: Update global_json_reports_list | ||
| ansible.builtin.set_fact: | ||
| global_json_reports_list: "{{ global_json_reports_list | default([]) + [junit2json_output_report_path] }}" |
There was a problem hiding this comment.
I also don't see an answer to this question, please answer before resolving the convos.
More feedback: you could put global_json_reports_list in your defaults file and you won't have to use default() here or pass [] as a variable
roles/junit2json/README.md
Outdated
| - name: "Example playbook to use the role redhatci.ocp.junit2json role" | ||
| hosts: localhost | ||
| vars: | ||
| global_json_reports_list: [] |
There was a problem hiding this comment.
this vars line is a bad example, because the nature of passing vars to a role makes it so every task inside the role has the same var definition block, every task receives an empty list for this variable, please remove
ae6416c to
38f057e
Compare
|
@thekad I think I covered everything. |
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 46s |
roles/junit2json/tasks/merge.yml
Outdated
| changed_when: | ||
| - true | ||
|
|
||
| - name: Validate some variables |
There was a problem hiding this comment.
Hi Max,
Few comments here please -
- The task name does not specify the real intention of this task.
- I would put this assertion somewhere at the beginning of the role execution, so that if this package is missing it will fail at an early stage (as @thekad) wrote.
- To me it looks redundant to register a variable to represent package cmd, there are more efficient, readable methods to do this, WDYT?
The role converts reports into json, optionally merging multiple fragments into 1 report in loadable json format
- added:
- role code
- initial tests
- example playbook
- role `README.md` using:
docsible -g -nob -nop -r roles/junit2json -p roles/junit2json/examples/example_playbook.yml
Signed-off-by: Maxim Kovgan <makovgan@redhat.com>
Updates to `redhat.ocp` collection:
- `galaxy.yml` - upd author and missing tag (fix `ansible-lint` issue)
- `README.md` at collection level
- filter plugin `redhatci.ocp.junit2obj`
- role `redhatci.ocp.junit2json`
- create `execution-environment.yml` to specify the collection requirements
- add 2 requirements files
Signed-off-by: Maxim Kovgan <makovgan@redhat.com>
|
Build succeeded. ✔️ dci-rpm-build-el8 SUCCESS in 2m 50s |
the role converts junit reports into json.
SUMMARY
Motivation:
As a part of group's desire to be able to make decisions based on data, we're investing into CI/Test observability.
The observability effort requires data collection and analysis, currently using Splunk with native data format as JSON + some data tagging/enrichment needs.
Current test reports are junit XML, which therefore needs to be converted and sent to a splunk index.
This role
junit2jsonis handling reports data conversion, using the previously mergedjson2objfilter.After merging it we will submit another one for sending the test reports, it should call this one to convert the data and send the resulting
.jsonfiles as splunk data frames.Fixes: TELCOV10N-452, TELCOV10N-451
ISSUE TYPE
Tests
collections/ansible_collections/redhatci/ocphierarchyansible-playbook collections/ansible_collections/redhatci/ocp/roles/junit2json/tests/test.ymlDepends-on: #510