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

http: feature/xml systemout #720

Merged
merged 13 commits into from
Nov 29, 2023
Merged

Conversation

ivan-velasco
Copy link
Contributor

http: addition of 'systemout' to the XML report for enhanced debugging capabilities, eliminating the need to solely rely on 'INFO' messages or enabling verbose mode after a complete run. With this enhancement, 'systemout' will be included in the XML output specifically when errors occur, improving the troubleshooting and diagnostic process.

Copy link
Member

@yesnault yesnault left a comment

Choose a reason for hiding this comment

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

Hi, thank you for this proposal. I'm not sure about the performance impact with big payload, but yes, this could be useful. LGTM.

@yesnault
Copy link
Member

After re-reading, I think that the body will be present twice in the JSON output. Maybe we can avoid that with the json annotation to '-'.

@@ -68,6 +68,7 @@ type Result struct {
BodyJSON interface{} `json:"bodyjson,omitempty" yaml:"bodyjson,omitempty"`
Headers Headers `json:"headers,omitempty" yaml:"headers,omitempty"`
Err string `json:"err,omitempty" yaml:"err,omitempty"`
Systemout string `json:"systemout,omitempty" yaml:"systemout,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Systemout string `json:"systemout,omitempty" yaml:"systemout,omitempty"`
Systemout string `json:"-" yaml:"-"`

Copy link
Member

Choose a reason for hiding this comment

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

ok with that @ivan-velasco ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to test this change out locally today @yesnault

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yesnault tested this finally today and it causes the xml report to exclude this information
When the property is set Systemout string json:"-" yaml:"-"

<system-out><![CDATA[

json report

"systemout": "\n",

When the property is set Systemout string json:"systemout,omitempty" yaml:"systemout,omitempty", then xml report shows up properly

<system-out><![CDATA[
PATCH-----http://

json report

"result.systemout": "PATCH-----http://
"systemout": "PATCH-----http://

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yesnault Any thoughts on this? Has been very helpful in identifying errors quickly.

@yesnault yesnault merged commit 0e8d78e into ovh:master Nov 29, 2023
2 checks passed
@ivan-velasco ivan-velasco deleted the feature/xml-systemout branch April 25, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants