[6.3] Restore activation key and content hosts subscriptions (BZ:1296978) #5374

ldjebran · 2017-10-17T09:01:58Z

cover: https://bugzilla.redhat.com/show_bug.cgi?id=1296978

(sat-6.3.0) dlezz@elysion:~/projects/robottelo-fork$ pytest -v tests/foreman/cli/test_subscription.py::SubscriptionTestCase::test_positive_restore_ak_and_content_hosts_subscriptions
================================================= test session starts ==================================================
platform linux2 -- Python 2.7.13, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /home/dlezz/.pyenv/versions/sat-6.3.0/bin/python2.7
cachedir: .cache
rootdir: /home/dlezz/projects/robottelo-fork, inifile:
plugins: xdist-1.20.0, services-1.2.1, mock-1.6.2, forked-0.2, cov-2.5.1
collected 1 item 
2017-10-17 10:51:47 - conftest - DEBUG - Deselect of WONTFIX BZs is disabled in settings


tests/foreman/cli/test_subscription.py::SubscriptionTestCase::test_positive_restore_ak_and_content_hosts_subscriptions PASSED

============================================== 1 passed in 656.55 seconds ==============================================

codecov · 2017-10-17T09:12:12Z

Codecov Report

Merging #5374 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #5374   +/-   ##
=======================================
  Coverage   61.32%   61.32%           
=======================================
  Files          33       33           
  Lines        3669     3669           
=======================================
  Hits         2250     2250           
  Misses       1419     1419

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c4625d...568786e. Read the comment docs.

renzon

ACK

oshtaier · 2017-11-10T08:36:02Z

tests/foreman/cli/test_subscription.py

+        :id: a44fdeda-9c8c-4316-85b4-a9b6b9f1ffdb
+
+        :steps:
+            1. Setup activation key , lifecycle environment and content view


Where did you get these steps? I read BZ information and it has some deviations from your steps

Especially that csv was edited to another host/capsules before applying

@oshtaier to make the exact BZ steps we have to be able to get an other manifest with the same subscriptions but with different agreement number, that is not realizable with the current infra implementation (we need muti-manifests s to be supported), the current coverage implement that by deleting completely the manifest and re-import all data, if you insist on the exact implementation we have to make this PR in do not merge state (marked as blocked ) and wait that PR implementation (multi-manifest) to be merged.

Hmm. Coverage that you provided in that PR is also needed, but still I am not sure that it is actually cover mentioned BZ

+1 to @oshtaier . The steps are looking different to me especially without changing subscription id in csv.
We need @JacobCallahan approval whether these steps fully cover the BZ, and if not - corresponding framework expansion is needed.

@abalakh @ldjebran There is one primary case that should be covered in the BZ, which the current test does not cover. The ideal steps for the bug would be:

Import a manifest, with a couple of subscriptions.

Register at least one host (preferably more).

Attach the same subscription to all registered hosts.

Export the content hosts to a csv file.

Modify the csv file to change the hosts from their current subscription to a different one.

Import the csv file
Result: The hosts should no longer have the old subscription, and now have the new one.

There is also an additional test case that could be written for what I did in comment 36. This is pretty similar to the steps above, but does not require hosts to be registered.

Upload a manifest with a couple of subscriptions.

Create an activation key, aligning one of the subscriptions to it.

Export the activation key to a csv file.

Modify the csv file to change the activation key's subscriptions from the previous sub to a different one.

Import the csv file.
Result: The activation key should not have the old subscription, and should have the new one.

With that said, this is still a very valid test that we should have in the code, just not aligned to this bug.

JacobCallahan · 2017-11-16T14:25:04Z

tests/foreman/cli/test_subscription.py

+        :id: a44fdeda-9c8c-4316-85b4-a9b6b9f1ffdb
+
+        :steps:
+            1. Setup activation key , lifecycle environment and content view


@abalakh @ldjebran There is one primary case that should be covered in the BZ, which the current test does not cover. The ideal steps for the bug would be:

Import a manifest, with a couple of subscriptions.

Register at least one host (preferably more).

Attach the same subscription to all registered hosts.

Export the content hosts to a csv file.

Modify the csv file to change the hosts from their current subscription to a different one.

Import the csv file
Result: The hosts should no longer have the old subscription, and now have the new one.

There is also an additional test case that could be written for what I did in comment 36. This is pretty similar to the steps above, but does not require hosts to be registered.

Upload a manifest with a couple of subscriptions.

Create an activation key, aligning one of the subscriptions to it.

Export the activation key to a csv file.

Modify the csv file to change the activation key's subscriptions from the previous sub to a different one.

Import the csv file.
Result: The activation key should not have the old subscription, and should have the new one.

With that said, this is still a very valid test that we should have in the code, just not aligned to this bug.

ldjebran · 2017-11-16T15:36:54Z

PUT dot merge to make the changes

ldjebran · 2017-11-21T08:49:55Z

@abalakh @JacobCallahan @oshtaier comments addressed

(sat-6.3.0) dlezz@elysion:~/projects/robottelo-fork$ pytest tests/foreman/cli/test_subscription.py -v -k "_restore_"
=================================================================================================== test session starts ====================================================================================================
platform linux2 -- Python 2.7.13, pytest-3.2.3, py-1.4.34, pluggy-0.4.0 -- /home/dlezz/.pyenv/versions/2.7.13/envs/sat-6.3.0/bin/python
cachedir: .cache
rootdir: /home/dlezz/projects/robottelo-fork, inifile:
plugins: xdist-1.20.1, services-1.2.1, mock-1.6.3, forked-0.2, cov-2.5.1, fauxfactory-1.0.1
collected 9 items                                                                                                                                                                                                           
2017-11-21 10:27:16 - conftest - DEBUG - BZ deselect is disabled in settings


tests/foreman/cli/test_subscription.py::SubscriptionTestCase::test_positive_restore_ak_and_content_hosts_subscriptions PASSED
tests/foreman/cli/test_subscription.py::SubscriptionTestCase::test_positive_restore_ak_with_modified_subscription PASSED
tests/foreman/cli/test_subscription.py::SubscriptionTestCase::test_positive_restore_content_hosts_with_modified_subscription PASSED

==================================================================================================== 6 tests deselected ====================================================================================================
============================================================

ldjebran · 2017-11-21T09:54:34Z

@renzon have added two more tests to this PR please revisit

renzon · 2017-11-21T13:00:44Z

ACK

JacobCallahan

ACK. These changes will be a fantastic addition to our automation!

oshtaier

Looks good

abalakh

ACK

abalakh · 2017-11-22T11:16:46Z

tests/foreman/cli/test_subscription.py

+            self.assertEqual(len(csv_data), 2)
+            for row_data in csv_data:
+                # The subscription is saved in the following format:
+                # """<quantity>|<sku>|<name>|<contract>|<account>"""


bonus karma for such comments, they're very helpful for understanding the code 👍

ldjebran added 6.3 CLI Issues and PRs involving the CLI QETestCoverage Issues and PRs relating to a Satellite bug labels Oct 17, 2017

ldjebran self-assigned this Oct 17, 2017

renzon approved these changes Nov 9, 2017

View reviewed changes

oshtaier reviewed Nov 10, 2017

View reviewed changes

oshtaier requested a review from abalakh November 15, 2017 11:27

abalakh requested a review from JacobCallahan November 15, 2017 16:20

JacobCallahan requested changes Nov 16, 2017

View reviewed changes

ldjebran added the DO NOT MERGE label Nov 16, 2017

ldjebran force-pushed the 6.3.0_qe_test_coverage_1296978 branch from 3bb54e7 to 41b1d9a Compare November 21, 2017 08:48

[6.3] Restore activation key and content hosts subscriptions

568786e

ldjebran force-pushed the 6.3.0_qe_test_coverage_1296978 branch from 41b1d9a to 568786e Compare November 21, 2017 08:51

ldjebran removed the DO NOT MERGE label Nov 21, 2017

ldjebran changed the title ~~[6.3] Restore activation key and content hosts subscriptions~~ [6.3] Restore activation key and content hosts subscriptions (BZ:1296978) Nov 21, 2017

JacobCallahan approved these changes Nov 21, 2017

View reviewed changes

oshtaier approved these changes Nov 22, 2017

View reviewed changes

abalakh approved these changes Nov 22, 2017

View reviewed changes

abalakh merged commit 70de63c into SatelliteQE:master Nov 22, 2017

ldjebran deleted the 6.3.0_qe_test_coverage_1296978 branch January 23, 2018 13:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[6.3] Restore activation key and content hosts subscriptions (BZ:1296978) #5374

[6.3] Restore activation key and content hosts subscriptions (BZ:1296978) #5374

ldjebran commented Oct 17, 2017

codecov bot commented Oct 17, 2017 •

edited

renzon left a comment

oshtaier Nov 10, 2017

oshtaier Nov 10, 2017

ldjebran Nov 13, 2017

oshtaier Nov 15, 2017

abalakh Nov 15, 2017

JacobCallahan Nov 16, 2017

JacobCallahan Nov 16, 2017

ldjebran commented Nov 16, 2017

ldjebran commented Nov 21, 2017

ldjebran commented Nov 21, 2017

renzon commented Nov 21, 2017

JacobCallahan left a comment

oshtaier left a comment

abalakh left a comment

abalakh Nov 22, 2017

[6.3] Restore activation key and content hosts subscriptions (BZ:1296978) #5374

[6.3] Restore activation key and content hosts subscriptions (BZ:1296978) #5374

Conversation

ldjebran commented Oct 17, 2017

codecov bot commented Oct 17, 2017 • edited

Codecov Report

renzon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ldjebran commented Nov 16, 2017

ldjebran commented Nov 21, 2017

ldjebran commented Nov 21, 2017

renzon commented Nov 21, 2017

JacobCallahan left a comment

Choose a reason for hiding this comment

oshtaier left a comment

Choose a reason for hiding this comment

abalakh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 17, 2017 •

edited