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

Add an extra escape wrapper for illegal XML chars in junit report plugin #3289

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Oct 15, 2024

The default Jinja escape (e) filter does not escape the XML illegal characters (like control characters). The python-xml-junit library was replacing the characters with an empty string:

For more info, please see:

Pull Request Checklist

  • implement the feature
  • extend the test coverage
  • include a release note

@seberm seberm added this to the 1.38 milestone Oct 15, 2024
@seberm seberm added plugin | junit The junit report plugin priority | must high priority, must be included in the next release ci | full test Pull request is ready for the full test execution labels Oct 15, 2024
@seberm seberm force-pushed the feature/escape-control-chars-in-junit branch 2 times, most recently from a7a6525 to 539e109 Compare October 15, 2024 13:32
@seberm seberm marked this pull request as ready for review October 15, 2024 13:36
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this! Looks good and works as expected.

@seberm
Copy link
Collaborator Author

seberm commented Oct 15, 2024

Thanks for fixing this! Looks good and works as expected.

Thanks for the review @psss !

Have you also tested the changes against the internal "base-os" tmt tests? Thanks.

@seberm
Copy link
Collaborator Author

seberm commented Oct 15, 2024

/packit test

@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 Oct 16, 2024
@psss
Copy link
Collaborator

psss commented Oct 16, 2024

Have you also tested the changes against the internal "base-os" tmt tests? Thanks.

Yes, before the change there's the traceback, with the change everything's green.

@psss psss force-pushed the feature/escape-control-chars-in-junit branch from 1f0b082 to f6ecb85 Compare October 16, 2024 13:17
@psss
Copy link
Collaborator

psss commented Oct 17, 2024

/packit test

@psss psss merged commit 91f838b into main Oct 17, 2024
20 of 22 checks passed
@psss psss deleted the feature/escape-control-chars-in-junit branch October 17, 2024 11:34
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 priority | must high priority, must be included in the next release status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants