pytest-testinfra icon indicating copy to clipboard operation
pytest-testinfra copied to clipboard

Improved RpmPackage is_installed property logic.

Open narmaku opened this issue 1 year ago • 2 comments

This new logic verifies if there is a "not installed" string in the stdout when "rpm -q %s" command returns 1.

As seen in bug #758, if rpm command failed because of other reasons (such as database corruption), the old implementation was returning True, making the end-user think that the package was installed, however it was never queried.

narmaku avatar Apr 01 '24 08:04 narmaku

@CarstenGrohmann , @philpep since you are the most active collaborators/maintainers, would you mind checking this quick PR? Thanks!

narmaku avatar May 01 '24 10:05 narmaku

lgtm, I'd just use f-strings, as testinfra requires Python>=3.9.
I'm also interested in learning about review process here, looks like there is quite a number of PRs without a response.

martinhoyer avatar May 01 '24 10:05 martinhoyer

LGTM, also. Please add a small test case.

CarstenGrohmann avatar May 06 '24 08:05 CarstenGrohmann

@narmaku: I suggest to simplify the test as a corrupt database is not necessary:

$ rpm -q not_installed_pkg
package not_installed_pkg is not installed
$ echo $?

CarstenGrohmann avatar May 13 '24 11:05 CarstenGrohmann

Merged, thanks!

philpep avatar May 26 '24 16:05 philpep