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

Add extended test for installonly packages #1470

Conversation

j-mracek
Copy link
Member

The original test is ok, but this test is for a random behavior of the code, therefore it uses more additional packages in test to triger random behavior.

Required for: https://issues.redhat.com/browse/RHEL-15902

Comment on lines 265 to 281
When I execute dnf with args "install abcde"
Then the exit code is 0
And Transaction is following
| Action | Package |
| install | abcde-0:2.9.2-1.fc29.noarch |
| install-dep | wget-0:1.19.5-5.fc29.x86_64 |
| install-weak | flac-0:1.3.2-8.fc29.x86_64 |
When I execute dnf with args "mark remove abcde"
Then the exit code is 0
And package reasons are
| Package | Reason |
| abcde-2.9.2-1.fc29.noarch | dependency |
| flac-1.3.2-8.fc29.x86_64 | weak-dependency |
| wget-1.19.5-5.fc29.x86_64 | dependency |
Copy link
Member

Choose a reason for hiding this comment

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

For the sake of future maintainers, I'd like to include a comment here explaining why the abcde package is needed on the system, with a dependency reason. I'm concerned that without it, anyone attempting to understand the test in a year or two might become confused.
What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point

Copy link
Member Author

Choose a reason for hiding this comment

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

I have created a comment.

@j-mracek j-mracek force-pushed the reason-installonly-extended branch from 93098a3 to be0f39f Compare March 12, 2024 07:32
| install-dep | wget-0:1.19.5-5.fc29.x86_64 |
| install-weak | flac-0:1.3.2-8.fc29.x86_64 |
# We need to have a different packages then kernel installed and with a different reasons then user to ensure that
# reason is inherited from expected package to expected package. The package abcd or its dependencies must be
Copy link
Member

Choose a reason for hiding this comment

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

This is sentence is a bit confusing.

Suggested change
# reason is inherited from expected package to expected package. The package abcd or its dependencies must be
# the "kernel" package reason is unexpectedly inherited from "abcde" package. The package abcd or its dependencies must be

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect, PR was updated.

The original test is ok, but this test is for a random behavior of the code,
therefore it uses more additional packages in test to triger random behavior.

Required for: https://issues.redhat.com/browse/RHEL-15902
@j-mracek j-mracek force-pushed the reason-installonly-extended branch from be0f39f to 23bea57 Compare March 12, 2024 13:29
Copy link
Member

@m-blaha m-blaha left a comment

Choose a reason for hiding this comment

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

Thank you!

@m-blaha m-blaha merged commit b2a97d2 into rpm-software-management:dnf-4-stack Mar 12, 2024
3 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.

None yet

2 participants