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

Integration Tests for DBus Call RegisterWithActivationKeys #3486

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jstavel
Copy link
Contributor

@jstavel jstavel commented Dec 20, 2024

Card-ID: CCT-1051

see https://www.candlepinproject.org/docs/subscription-manager/dbus_objects.html
It is a DBus Method RegisterWithActivationKeys.

This PR covers happy path and wrong path for registering with activation keys.

There can be even wrong organization in a case with activation keys.
The method should provides a useful response when something wrong was used in arguments of the method.

Copy link

github-actions bot commented Dec 20, 2024

Coverage

Coverage (computed on Fedora latest) •
FileStmtsMissCoverMissing
TOTAL17432446574% 
report-only-changed-files is enabled. No files were changed during this commit :)

Tests Skipped Failures Errors Time
2416 14 💤 0 ❌ 0 🔥 31.094s ⏱️

Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

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

Thanks for PR. Overall good, I have only two requests.


@pytest.mark.parametrize(
"wrong_case", ["wrong-act-key inside", "only wrong-act-key", "wrong-org", "wrong-act-key and wrong-org"]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use several test_ methods for each wrong case. This is hard to understand.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that it would be hard to use this approach: https://github.com/candlepin/subscription-manager/blob/main/integration-tests/test_register.py#L110, because returned error message is different for each wrong case.

@@ -42,7 +42,6 @@ EOF
./integration-tests/scripts/run-local-candlepin.sh

# create testing data in local candlepin
./integration-tests/scripts/post-activation-keys.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

This change should not be part of this PR. It is already part of another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will revert this change .

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