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
test_positive_end_to_end_bulk_update (#6956) #6974
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6974 +/- ##
=======================================
Coverage 69.76% 69.76%
=======================================
Files 29 29
Lines 3493 3493
=======================================
Hits 2437 2437
Misses 1056 1056 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test itself looks OK to me, I left couple of comments for things around it.
Also, I think it depends on Airgun PR that is not yet merged? Could you add reference to said PR in initial PR message?
|
5e1d1cf
to
67decc1
Compare
Fixed indentation in docstring |
Hello I installed Satellite 6.4 and upgraded it to Satellite 6.5. I then entered this command:
I then ran test which failed as expected because packages do not get updated due to BZ#1712069: robottelo]$ pytest -v tests/foreman/ui_airgun/test_contenthost.py -k test_positive_end_to_end_bulk_update collected 17 items / 16 deselected tests/foreman/ui_airgun/test_contenthost.py::test_positive_end_to_end_bulk_update FAILED [100%] ============================== FAILURES ============================= session = <airgun.session.Session object at 0x7f8b0c3ecb70>
E AssertionError: assert 'walrus-0.71-1.noarch' == 'walrus-5.21-1.noarch' tests/foreman/ui_airgun/test_contenthost.py:257: AssertionError I then upgraded Sat6.5 to latest snap of 6.6 and entered this command: I then ran the test again: robottelo]$ pytest --show-capture=no tests/foreman/ui_airgun/test_contenthost.py -k test_positive_end_to_end_bulk_update collected 17 items / 16 deselected tests/foreman/ui_airgun/test_contenthost.py . [100%] ================= 1 passed, 16 deselected, 2 warnings in 807.97 seconds ================= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All my comments are addressed. Looks good 👍
@swadeley can you confirm that these upgrades are just part of your testing environment, not actually required by the test? I mean - if you fresh-install 6.5, we can expect it to fail (due to bug), and if you fresh-install 6.6, we can expect it to pass (because bug is already fixed). Is that correct?
Because if you have to run upgrades as part of test scenario, we probably should put it somewhere else. Scenarios in tests/foreman
are also run for fresh install of each new snap.
@Mirzal The issue opened by @ntkathole says "Add upgrade test" but I am struggling to find evidence it is upgrade related. I will run the test against a fresh install of 6.5 and also ask SME to confirm bug is upgrade related (he was a away so could not ask before). But in any case, the upgrade is not part of the test, that was only to reproduce the bug so I could check the test finds it. But if we find its not upgrade related, then we could remove the tag More news soon |
It's up to you. |
Hello I have decided to add an extra step, to make sure package event can take place if there is slower system or network traffic. So blocking the merge for now. |
Thanks for that. I tracked it down, the source of the word "upgrade" is from an e-mail with subject "Unable to apply package update after upgrade from 6.4 to 6.5". But the SME, the developer, thinks the issue is not related to the fact that the Satellite was upgraded. To confirm I installed a new 6.5 system late yesterday and the fault was present, so I will remove that tag. |
Hey @swadeley please keep upgrade decorator. This will cover both the scenarios of fresh 6.5 install and upgraded 6.5. |
@ntkathole OK, keep upgrade decorator |
Hello I added wait_for_tasks with 10 sec poll time because my testing suggests we need to allow at least a minute for the client to complete the installation and do the upload profile. I also do not want to overload the server with searches that will not find anything. I can squash commits later or the merger can do it. New test results coming. Thank you |
robottelo]$ pytest --show-capture=no tests/foreman/ui_airgun/test_contenthost.py::test_positive_end_to_end_bulk_update ========================== test session starts ====================================== collected 1 item tests/foreman/ui_airgun/test_contenthost.py . [100%] ==================== 1 passed, 2 warnings in 704.79 seconds ===================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one change. Everything else looks good to me!
robottelo]$ pytest --show-capture=no tests/foreman/ui_airgun/test_contenthost.py::test_positive_end_to_end_bulk_update collected 1 item tests/foreman/ui_airgun/test_contenthost.py . [100%] =================== 1 passed, 5 warnings in 745.71 seconds ==================== |
* Test the Update All Packages button on Update Packages dialogue * Use the Host Collection view to get to it.
Hello, due to merge conflicts, I did |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
Edit: This test depends on the change in SatelliteQE/airgun#332