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

Return full error message from xmllint #586

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

peverwhee
Copy link
Collaborator

Update call_command function to include stderr output in CCPPError message.

Right now, when xmllint-ing is done via xml_tools.py (validate_xml_file),
the output of the linter is not returned. This PR adds the stderr output
(if any) to the returned error message.

PR also decodes error messages for cleaner output.

User interface changes?: No

Testing:
test removed: N/A
unit tests: Added new doctest to test error message
system tests: N/A
manual testing: Ran with/parsed new error messages within CAM-SIMA

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for fixing this

Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

The output looks incorrect if a command has output to both stdout and stderr.

scripts/parse_tools/xml_tools.py Show resolved Hide resolved
scripts/parse_tools/xml_tools.py Outdated Show resolved Hide resolved
scripts/parse_tools/xml_tools.py Outdated Show resolved Hide resolved
scripts/parse_tools/xml_tools.py Show resolved Hide resolved
Copy link
Collaborator

@gold2718 gold2718 left a comment

Choose a reason for hiding this comment

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

Looks great now, thanks!

Copy link
Collaborator

@dustinswales dustinswales left a comment

Choose a reason for hiding this comment

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

@peverwhee These changes look good to me. Thanks!

@climbfuji climbfuji merged commit 49a3c3f into NCAR:develop Oct 3, 2024
19 checks passed
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.

4 participants