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: add feature presence automation #7740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ikerexxe
Copy link
Contributor

@ikerexxe ikerexxe commented Dec 3, 2024

The test case can be further extended to cover other features by using the parametrization that is already in place.

@jakub-vavra-cz
Copy link
Contributor

Hi, this looks pretty good but there are some failures in the tests so we need to double check if the features should (not) be present.

@ikerexxe
Copy link
Contributor Author

ikerexxe commented Dec 5, 2024

Hi, this looks pretty good but there are some failures in the tests so we need to double check if the features should (not) be present.

I checked it with Alexey and the spec file to make sure everything is right. This is ready for review again.

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

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

LGTM

@jakub-vavra-cz
Copy link
Contributor

@ikerexxe I wonder if we want more parts something for 2.9.5-4.el9_5.2 etc. as some feature like exop_force might land in different versions across different rhel versions.

@ikerexxe
Copy link
Contributor Author

ikerexxe commented Dec 5, 2024

@ikerexxe I wonder if we want more parts something for 2.9.5-4.el9_5.2 etc. as some feature like exop_force might land in different versions across different rhel versions.

That's what RHEL major and minor versions are for. Or do you mean some other thing?

Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

I think this is good for a quick assertion to ensure the feature is present or not. I did like @jakub-vavra-cz idea to associate the condition in the test, so it will skip or run, not just feature checking. Maybe we can return to that topic next year.

So just the minor nitpick comments, thank you Iker.

src/tests/system/tests/test_feature_presence.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_feature_presence.py Outdated Show resolved Hide resolved
The test case can be further extended to cover other features by using
the parametrization that is already in place.

Signed-off-by: Iker Pedrosa <[email protected]>
@ikerexxe
Copy link
Contributor Author

@danlavu @jakub-vavra-cz I would like to have this merged before the vacation period, so please do a final review. We can always expand the test case in the future to include additional information as needed.

@danlavu danlavu self-requested a review December 13, 2024 14:13
Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

LG2M

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants