Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for junit-xml, use Jinja2 instead #3150

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Aug 15, 2024

This PR removes support for the deprecated junit-xml library and uses the Jinja2 to generate JUnit XML instead.

It also prepares support for various junit flavors (--flavor) and adds the possibility to specify user-defined JUnit Jinja templates using the --template-path and --flavor=custom options. These options are mutually exclusive.

The current implementation requires that custom templates must be valid XML files. If we want to allow users use non-XML custom templates, we would probably go with something like template report plugin for results.

/plans/example<br>
    report
        how: junit
        warn: The 'custom' JUnit flavor is used, you are solely responsible for the validity of the XML schema.
        warn: The pretty print is always disabled for 'custom' JUnit flavor
        warn: The generated XML output is not valid XML file or it is not valid against the XSD schema.
        warn: <string>:1:1:FATAL:PARSER:ERR_DOCUMENT_EMPTY: Start tag expected, '<' not found
        warn: Start tag expected, '<' not found, line 1, column 1 (<string>, line 1)

plan failed

The exception was caused by 1 earlier exceptions

Cause number 1:

    The generated XML output is not a valid XML file.

The implementation of the polarion flavor and its support in the Polarion report plugin will be addressed in a separate PR. In this PR, the Polarion report plugin just calls the junit.make_junit_xml function in nearly the same way as before, aiming to keep the Polarion xunit.xml file unchanged.

TODOs:

  • Probably divide the changes into multiple PRs (polarion and junit separately)
  • Define XSD JUnit for default flavor and validate the XML output against this schema. Warn user if there are any problems.
  • [ ] Possibility to specify template variables directly from cmdline as key-values (e.g. --var <key> <value>) for a custom flavor / user-defined template? - This could be a part of "template" report plugin in the future.
  • The difference between junit-xml generated files and the new junit files is that the quotes " are not escaped to &quot; inside of tag data (they are correctly escaped inside attributes). According to my knowledge, this should be OK and the XML is also valid.
  • Add tests
  • Add test which checks there is no schema/XML warning in the tmt logs when running non-custom flavor The generated XML output is not a valid XML file or it is not valid against the XSD schema.
  • The duration should be probably converted to float and propagated to XML time="<val>" attributes as float, not an integer. If this gets changed, is it possible to enforce float attributes in XSD schema? - tmt works with seconds, let's just convert them to floats using a jinja filter.

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

@seberm seberm self-assigned this Aug 15, 2024
@seberm seberm added the step | report Stuff related to the report step label Aug 15, 2024
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
@seberm seberm added plugin | polarion Plugins implementing the Polarion integration plugin | junit The junit report plugin labels Aug 15, 2024
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch from 7b86542 to be139fe Compare August 15, 2024 21:51
@seberm
Copy link
Collaborator Author

seberm commented Aug 15, 2024

Hello @happz , thanks for your early hints. It's still very early for a review. Many code parts will probably change, especially due to the Polarion report plugin.

@LecrisUT
Copy link
Contributor

Is it possible to validate the junit.xml files created?

@seberm
Copy link
Collaborator Author

seberm commented Aug 19, 2024

Is it possible to validate the junit.xml files created?

Currently, this functionality is not supported, and the XML files are not validated against any XSD schema, even when using junit-xml.

From python-junit-xml README.rst:

As there is no definitive Jenkins JUnit XSD that I could find, the XML documents created by this module support a schema based on Google searches and the Jenkins JUnit XML reader source code. File a bug if something doesn't work like you expect it to. For Bamboo situation is the same.

The junit-xml repository currently includes only Python unit tests, while TMT tests have very basic XML checks (polarion, default junit). However, there is no existing code that validates the entire XML file against an XSD schema.

I plan to create simple XSD schemas for each JUnit flavor supported by TMT and add tests validating the generated JUnit files against these schemas.

Additionally, it would be a good idea to have TMT dynamically validate the schema of the generated JUnit files. If a file does not conform to its respective schema, the report plugin should fail, unless e.g. the --force argument is used, which would allow it to continue despite the schema mismatch.

@happz
Copy link
Collaborator

happz commented Aug 19, 2024

I plan to create simple XSD schemas for each JUnit flavor supported by TMT and add tests validating the generated JUnit files against these schemas.

Additionally, it would be a good idea to have TMT dynamically validate the schema of the generated JUnit files. If a file does not conform to its respective schema, the report plugin should fail, unless e.g. the --force argument is used, which would allow it to continue despite the schema mismatch.

+1. Check out #3153, it does similar validation when consuming results.yaml, but it's not a blocker, only a warning sign, at least for now. That said, these files are not created by tmt - junit files would be. Failing this particular validation would mean the plugin is creating a broken output, which would be a bug. I'm not sure --force would be needed - if users wanted to overcome this bug temporarily, they should be able to modify either the schema or the template, and --force would sort of normalize the behavior in my eyes.

@seberm seberm force-pushed the feature/generate-junit-using-jinja branch 6 times, most recently from a2a6e2b to 47307a9 Compare August 19, 2024 18:04
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch 4 times, most recently from b042929 to 1e59194 Compare August 21, 2024 08:21
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
@happz happz modified the milestones: 1.36, 1.37 Sep 3, 2024
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch from 280830b to 85b1822 Compare September 3, 2024 12:02
docs/releases.rst Outdated Show resolved Hide resolved
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch 2 times, most recently from 0d2969b to 8554165 Compare September 3, 2024 13:19
tests/unit/test_report_junit.py Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
tmt/steps/report/junit.py Outdated Show resolved Hide resolved
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch 3 times, most recently from 9ee1638 to e613189 Compare September 6, 2024 11:35
@seberm seberm requested a review from thrix September 9, 2024 11:07
@happz happz force-pushed the feature/generate-junit-using-jinja branch from e613189 to d8c650a Compare September 10, 2024 09:05
Copy link
Collaborator

@thrix thrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Sep 11, 2024
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch from d8c650a to 49a0604 Compare September 11, 2024 09:24
@seberm seberm force-pushed the feature/generate-junit-using-jinja branch from 49a0604 to 597e000 Compare September 12, 2024 11:32
@happz happz force-pushed the feature/generate-junit-using-jinja branch from 597e000 to 38c9d87 Compare September 12, 2024 13:01
@psss psss merged commit 9919b75 into main Sep 13, 2024
21 of 22 checks passed
@psss psss deleted the feature/generate-junit-using-jinja branch September 13, 2024 16:29
The-Mule pushed a commit to The-Mule/tmt that referenced this pull request Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution plugin | junit The junit report plugin plugin | polarion Plugins implementing the Polarion integration status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. step | report Stuff related to the report step
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants