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.

Sign up for GitHub

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

Jump to bottom

Quote values in the tmt-report-results.yaml file #3231

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Quote values in the tmt-report-results.yaml file #3231

merged 2 commits into from
Sep 24, 2024

Conversation

seberm
Copy link
Collaborator

@seberm seberm commented Sep 22, 2024 •
edited
Loading

Put all the generated values inside quotation marks in the tmt-report-results.yaml YAML file.

Try to fix:

Pull Request Checklist

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

@seberm seberm self-assigned this Sep 22, 2024
@seberm seberm added ci | full test Pull request is ready for the full test execution step | execute Stuff related to the execute step area | restraint Backward compatibility support for restraint labels Sep 22, 2024
@seberm seberm force-pushed the feature/escape-tmt-report-results branch from 3fcb607 to afcca90 Compare September 23, 2024 07:32
@seberm seberm mentioned this pull request Sep 23, 2024
Closed
@psss psss added this to the 1.37 milestone Sep 23, 2024
@psss psss added the priority | must high priority, must be included in the next release label Sep 23, 2024
@psss
Copy link
Collaborator

psss commented Sep 23, 2024

This is actually fixing a regression, marking as a must for 1.37.

@psss psss linked an issue Sep 23, 2024 that may be closed by this pull request
tmt-report-result: may need to escape special symbols like colon #3226
Closed
@seberm seberm force-pushed the feature/escape-tmt-report-results branch from afcca90 to 34539c1 Compare September 23, 2024 17:20
@seberm seberm requested a review from psss September 23, 2024 17:22
psss
psss requested changes Sep 24, 2024
View reviewed changes
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 much for fixing this! Looks good, added a few comments and adjusted release not width in 9f78c35.

tests/execute/result/special.sh Show resolved Hide resolved
tests/execute/result/special/special-chars.fmf Show resolved Hide resolved
tmt/steps/execute/scripts/tmt-report-result Show resolved Hide resolved
@seberm seberm force-pushed the feature/escape-tmt-report-results branch from 9f78c35 to 4a7311b Compare September 24, 2024 10:06
psss
psss approved these changes Sep 24, 2024
View reviewed changes
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.

Now looks good, thanks!

@psss psss changed the title Add quotation marks around values in tmt-report-result output Quote values in the tmt-report-results.yaml file Sep 24, 2024
thrix
thrix approved these changes Sep 24, 2024
View reviewed changes
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

@psss psss force-pushed the feature/escape-tmt-report-results branch from e9cdf41 to 9c59c33 Compare September 24, 2024 18:49
@psss psss 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 24, 2024
@psss psss merged commit 6e5b8b3 into main Sep 24, 2024
23 checks passed
@psss psss deleted the feature/escape-tmt-report-results branch September 24, 2024 21:28
@psss psss assigned psss and unassigned seberm Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thrix thrix thrix approved these changes

@psss psss psss approved these changes

@therazix therazix therazix approved these changes

@lukaszachy lukaszachy Awaiting requested review from lukaszachy lukaszachy is a code owner

@happz happz Awaiting requested review from happz happz is a code owner

@janhavlin janhavlin Awaiting requested review from janhavlin janhavlin is a code owner

Assignees

@psss psss

Labels
area | restraint Backward compatibility support for restraint ci | full test Pull request is ready for the full test execution 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. step | execute Stuff related to the execute step
Projects
None yet
Milestone
1.37
Development

Successfully merging this pull request may close these issues.

tmt-report-result: may need to escape special symbols like colon
4 participants
@seberm @psss @thrix @therazix