openSUSE / obs-service-replace_using_package_version

An OBS service to replace a regex with some package version available in the build time repositories
GNU General Public License v3.0
4 stars 12 forks source link

Ignore stderr on successful command executions #35

Closed davidcassany closed 1 year ago

davidcassany commented 1 year ago

This fixes a regression from 43e56dd1. On success, the command output should only include the stdout, otherwise we get the normalized output of rpm queries polluted with potential warning messages.

In case of error python's subprocess will already rise an Exception with stdout, sdterr and return code.

Signed-off-by: David Cassany dcassany@suse.com

davidcassany commented 1 year ago

@dcermak I'll try to quickly figure out if I can improve the unit tests to catch this sort of issues, not sure I'll manage, I don't have much time to spend on it.

davidcassany commented 1 year ago

All right the image is building again here https://build.opensuse.org/project/show/home:dcassany:branches:openSUSE:Factory:Staging:E I could not find a simple way to mock subprocess behavior other than mocking check_output directly, so check_output behavior is still untested here, so we are not prevented to catch similar issues by simply relaying on unit tests.

For the records, inside OBS, rpm -qp <rpm_file> throws a warning message of unsigned packages (as it happens on zypper install when running KIWI inside OBS). This warning is sent to stderr, hence we need to ignore the standard error to parse the the result with the requested query format.

davidcassany commented 1 year ago

LGTM, thanks for the fix!

You are welcome, thanks also for the integration test. Those are the little details hard to catch on tests and on reviews

davidcassany commented 1 year ago

Gonna merge it once the integration tests is green and bump version again and then submit again to OBS.