Add create xml results file to validations role #2456
Conversation
|
Hi @drosenfe. Thanks for your PR. I'm waiting for a openstack-k8s-operators member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Thanks for the PR! ❤️ |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a5539da889374d38806c95d594d89b53 ✔️ noop SUCCESS in 0s |
drosenfe
force-pushed
the
createxmlfile
branch
from
598e121
to
8bd0555
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/cf725184fc7a490aacc910ad4cdc6489 ✔️ noop SUCCESS in 0s |
drosenfe
force-pushed
the
createxmlfile
branch
from
8bd0555
to
f9e85c5
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/31370ac9596b472daa36cebad50d2757 ✔️ noop SUCCESS in 0s |
|
recheck |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/df2950cb83144d738cf509d8362894e0 ✔️ noop SUCCESS in 0s |
drosenfe
force-pushed
the
createxmlfile
branch
from
f9e85c5
to
b29f3bd
Compare
|
/ok-to-test |
There was a problem hiding this comment.
If this is working to get the data you need into Polarion then it lgtm. Just some follow up tidying that we can do, but realistically it's orthogonal to this change anyway. I don't want to distract from the objective of the PR with that conversation here, this change just draws attention to that requirement for me.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lewisdenny The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I think ultimately, what might be nice as a follow up to this PR is a new task file in the polarion role that will take some Ansible variable inputs, and use a jinja2 template to format those results into the required XML for polarion. Then instead of all the XML handling we're doing in this PR, we can simply call import tasks_from that new polarion file and it can handle to XML side of things. But since this is only going to impact the EDPM validations, it's fairly limited impact if something were to stop working for some reason. I'm in favor of merging it to keep progress going, and maybe we can backlog a Epic to tidy up the XML side of things. |
| - name: Write first line of xml results file | ||
| ansible.builtin.shell: | | ||
| mkdir -p "{{ cifmw_validations_xml_status_file_dir }}" | ||
| echo "<testsuite errors=\"{{ validations_errors }}\" failures=\"{{ validations_failed }}\" name=\"\" tests=\"{{ validations_executed }}\" time=\"{{ validations_run_time }}\">" >> {{ cifmw_validations_xml_status_file_dir }}/validations_results.xml |
There was a problem hiding this comment.
Generating XML with bash is asking for trouble. There's not nice way to generate XML in Ansible, but using jinja2 would be a step forward.
There was a problem hiding this comment.
@pablintino Is this comment a goal or a requirement? Before I started this review I asked on the proj-create-integration-pipeline-jobs about creating xml files and the answer was: it could be even a simple set of echo "" , but if you want to use something easy and more structured you could use https://pypi.org/project/junitparser/
There was a problem hiding this comment.
@drosenfe It's a suggestion to improve the maintainability of the role, specially given that I don't see a molecule test that ensure the rendered XML matches the expectations. If you and all the ones involved in this role are fine I'm ok with the change.
There was a problem hiding this comment.
I looked quite a lot at junitparser. Doing a fake test from command line this is the output it gives:
<?xml version="1.0" encoding="utf-8"?>
<testsuites>
<testsuite name="validations" tests="3" errors="0" failures="0" skipped="0" time="1.8">
<testcase name="hugepages_and_reboot.yml" classname="validations.edpm" time="0.5"/>
<testcase name="hotfix.yml" classname="validations.edpm" time="0.6"/>
<testcase name="scaledown.yml" classname="validations.edpm" time="0.7"/>
</testsuite>
</testsuites>
This different in a few ways from the current xml files generated by tempest
- it uses tabs instead of spaces (could be changed with sed after creation)
- it has an extra xml header (could be deleted after creation)
- the test cases have order: name, classname, time while tempest xml files have order: classname, name, time
- the testsuite order for tests, errors, failures, skipped is different
What does anyone think about using junitparser? There were some advantages. It sums the test cases, failures, and total time so that the validations roles does not need too track those.
| block: | ||
| - name: Make new xml line list entry | ||
| ansible.builtin.set_fact: | ||
| new_xml_line_list_entry: [' <testcase classname="validations.{{item | dirname}}" name="{{item | basename}}" time="{{ individual_validator_run_time }}">'] |
There was a problem hiding this comment.
Details like the indentation handling you do here is one of the reasons I added this comment:
https://github.com/openstack-k8s-operators/ci-framework/pull/2456/files#diff-745f651c7f88527b40991eca9fbe69b880b93734ee5a24b140a3fcac8becac2aR87
drosenfe
force-pushed
the
createxmlfile
branch
from
b29f3bd
to
03c4264
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/50249f51bb214b8a889635e7f9373b0d ✔️ noop SUCCESS in 0s |
drosenfe
force-pushed
the
createxmlfile
branch
from
03c4264
to
bf9980e
Compare
Add generation of xml results file to validations role. This will allow polarion status reporting for validations.