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

[6.3] Reinstall capsule on the same node after remove (BZ: 1327442) #5388

Merged
merged 1 commit into from Oct 23, 2017

Conversation

ldjebran
Copy link
Contributor

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

(sat-6.3.0) dlezz@elysion:~/projects/robottelo-fork$ pytest -v tests/foreman/cli/test_capsule_installer.py::CapsuleInstallerTestCase::test_positive_reinstall_on_same_node_after_remove
================================================= 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-19 14:34:08 - conftest - DEBUG - Deselect of WONTFIX BZs is disabled in settings


tests/foreman/cli/test_capsule_installer.py::CapsuleInstallerTestCase::test_positive_reinstall_on_same_node_after_remove <- robottelo/decorators/__init__.py PASSED

============================================= 1 passed in 5900.26 seconds ==============================================

@ldjebran ldjebran added 6.3 CLI Issues and PRs involving the CLI QETestCoverage Issues and PRs relating to a Satellite bug labels Oct 19, 2017
@ldjebran ldjebran self-assigned this Oct 19, 2017
@codecov
Copy link

codecov bot commented Oct 19, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5388   +/-   ##
=======================================
  Coverage   60.67%   60.67%           
=======================================
  Files          34       34           
  Lines        3697     3697           
=======================================
  Hits         2243     2243           
  Misses       1454     1454

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 9c67322...a9efad0. Read the comment docs.

Copy link
Contributor

@oshtaier oshtaier left a comment

Choose a reason for hiding this comment

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

ACK

Copy link
Contributor

@pondrejk pondrejk left a comment

Choose a reason for hiding this comment

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

looking at the bz, wouldn't it be less expensive to check if all appropriate files were deleted after remove, rather than installing the capsule again?

@pondrejk
Copy link
Contributor

Or differently, we could assert that configs were removed after katello-remove finishes, this way if something slips, we wouldn't have to wait for failing capsule-installer to find out, does it make sense?

Copy link
Contributor

@abalakh abalakh left a comment

Choose a reason for hiding this comment

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

ACK pending @pondrejk suggestion - assert config files are absent before reinstalling the capsule. And also your test takes around 99 minutes, pls move it to tier4 instead of tier3

@ldjebran
Copy link
Contributor Author

ldjebran commented Oct 23, 2017

The BZ say that the install failed because of existing configs and certs.

The main BZ complains is about install failure after remove. So install after remove is our goal to track

@abalakh @pondrejk this is not a test coverage of katello-remove
If it was the case we have to check all the packages, files and certs was removed we have also to check that all services are not working , but this is not the case

This is a test coverage of install after remove command

@ldjebran
Copy link
Contributor Author

@abalakh @pondrejk even if we check that all the configs where removed, we have no guaranty that the capsule will reinstall successfully , so the only way to ensure that, is to reinstall it for real

@abalakh
Copy link
Contributor

abalakh commented Oct 23, 2017

@ldjebran you probably didn't get me right. I'm not asking about removing reinstall part of the test (actually i'm against it, that's a nice coverage which we hadn't before), but proposing to add one extra assertion. If configs were not removed then reinstall won't succeed so there's no point in proceeding with scenario. Consider it smth like our self.assertEqual(result.return_code, 0) asserts - which aren't the things we need to verify in our tests but are saving us time since there's no point in proceeding when we know already that one of key commands has failed.

Copy link
Contributor

@abalakh abalakh left a comment

Choose a reason for hiding this comment

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

Ok, nevermind, as the test is called test_positive_reinstall_on_same_node_after_remove maybe you have a point, it's not a bad idea to test reinstall no matter what's happening on lower level, were or were not some configs removed. Basically, even if they were not removed but capsule installation succeeded - we're good to mark the test with 'PASS'.

ACK

@abalakh abalakh merged commit c9b9364 into SatelliteQE:master Oct 23, 2017
@ldjebran ldjebran deleted the 6.3.0_qe_test_coverage_1327442 branch January 23, 2018 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Issues and PRs involving the CLI QETestCoverage Issues and PRs relating to a Satellite bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants