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

tests: Don't print parsing errors during tests #107

Merged
merged 2 commits into from
Oct 17, 2023

Conversation

pkratoch
Copy link
Contributor

@pkratoch pkratoch commented Oct 5, 2023

The tests are unnecessarily chatty and print many errors that are actually expected. In case of unexpected errors, it wouldn't print all of them anyway, and would need to be enabled, since there are other parsers initialized to not print the logs.

The tests are unnecessarily chatty and print many errors that are actually
expected. In case of unexpected errors, it wouldn't print all of them
anyway, and would need to be enabled, since there are other parsers
initialized to not print the logs.
@pkratoch pkratoch mentioned this pull request Oct 5, 2023
@jan-kolarik jan-kolarik self-assigned this Oct 16, 2023
@jan-kolarik
Copy link
Member

Thanks for that. Although there is still a DTD validating involved, which is sadly doing the printing on its own, it would need some effort and API modifications to make it consistent with other comps_parse_* functions.

@jan-kolarik
Copy link
Member

jan-kolarik commented Oct 16, 2023

Thanks for that. Although there is still a DTD validating involved, which is sadly doing the printing on its own, it would need some effort and API modifications to make it consistent with other comps_parse_* functions.

@pkratoch Or if you think we can just drop that printf from there? Because it's now used just in the unit test within libcomps. And the headers are IIUC just for developmental purposes.

@pkratoch
Copy link
Contributor Author

Thanks for that. Although there is still a DTD validating involved, which is sadly doing the printing on its own, it would need some effort and API modifications to make it consistent with other comps_parse_* functions.

@pkratoch Or if you think we can just drop that printf from there? Because it's now used just in the unit test within libcomps. And the headers are IIUC just for developmental purposes.

Hm, I am not sure what all components use this and if removing the printf wouldn't cause problems elsewhere. It seems safer to print the error during tests than not to print the error when there would be actual problem.
And regarding the modification to respect the log->std_out, I am not sure if the effort is worth it, especially given how low severity the issue is.

@jan-kolarik jan-kolarik merged commit 220078b into rpm-software-management:master Oct 17, 2023
4 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.

2 participants