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]register twice with uppercase chars in hostname (BZ1361309) #5311

Merged
merged 1 commit into from Sep 26, 2017

Conversation

ldjebran
Copy link
Contributor

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

(sat-6.3) dl@DL:~/projects/redhat/robottelo$ pytest -v tests/foreman/cli/test_host.py::HostCreateTestCase::test_positive_register_twice_with_uppercase_chars_in_hostname
================================================= test session starts ==================================================
platform linux2 -- Python 2.7.13, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /home/dl/.pyenv/versions/sat-6.3/bin/python
cachedir: .cache
rootdir: /home/dl/projects/redhat/robottelo, 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-09-20 16:24:44 - conftest - DEBUG - Found WONTFIX in decorated tests ['1156555', '1269196', '1378009', '1245334', '1217635', '1226425', '1147100', '1311113', '1199150', '1204686', '1221971', '1103157', '1230902', '1230865', '1214312', '1079482']

2017-09-20 16:24:44 - conftest - DEBUG - Collected 1 test cases


tests/foreman/cli/test_host.py::HostCreateTestCase::test_positive_register_twice_with_uppercase_chars_in_hostname PASSED

================================================== 0 tests deselected ==================================================
============================================== 1 passed in 304.08 seconds ==============================================

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

codecov bot commented Sep 20, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5311   +/-   ##
=======================================
  Coverage   60.89%   60.89%           
=======================================
  Files          34       34           
  Lines        3659     3659           
=======================================
  Hits         2228     2228           
  Misses       1431     1431

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 68e229d...23cfcad. 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.

Basically, it was another idea. Maybe, I read you code wrong, but you should cover not register host twice, but register host with name like 'aaaAAAaaa' and then 'AAAAAAAAA', but you register 'AAAAAAAAA' twice. That is completely another thing, so please re-implement test case to that scenario.

And it is completely understandable why it was failing in the first place. I doubt that re-registering is going fail at all

@oshtaier
Copy link
Contributor

Yeah. I checked the PR to the code base. They meant specifically what I meant in my comment above

@oshtaier
Copy link
Contributor

and it should not be lower() and then upper(), just normal name and then upper()

@ldjebran
Copy link
Contributor Author

ldjebran commented Sep 20, 2017

@oshtaier please point to the description that host name should be changed between registrations,
we can add an other test for your case , but the bug it self does not say that
it say because sat register hostname in lowercase it thinks that the host in an other host, raise error that that name is already taken

@ldjebran
Copy link
Contributor Author

ldjebran commented Sep 20, 2017

@oshtaier the bug description

If a host has a hostname with any uppercase letters, re-registration will try to find it, but not be able to because its stored with lowercase letters.

is there some think I cannot understand here ???
bug steps:

Steps to reproduce:
1) hostname 'FooBar.example.com'
2) subscription-manager register
3) subscription-manager register --force

where is changing hostname ???

@ldjebran
Copy link
Contributor Author

@oshtaier I do not agree with you.
if you want an other test that cover your scenario , no problem.

@oshtaier
Copy link
Contributor

Okay, if you want to do in that way then why do you do target_image = gen_string('alpha').upper()?, use similar name to one in example: 'MyTest.test.com'?

@ldjebran
Copy link
Contributor Author

Ok no problem with the name, I will change that to some thing like AbCdEf

@oshtaier
Copy link
Contributor

Thanks!

@ldjebran
Copy link
Contributor Author

@oshtaier updated the name generated for test was "KfMtWeXaHa"

(sat-6.3) dl@DL:~/projects/redhat/robottelo$ pytest -v tests/foreman/cli/test_host.py::HostCreateTestCase::test_positive_register_twice_with_uppercase_chars_in_hostname
================================================= test session starts ==================================================
platform linux2 -- Python 2.7.13, pytest-3.1.3, py-1.4.34, pluggy-0.4.0 -- /home/dl/.pyenv/versions/sat-6.3/bin/python
cachedir: .cache
rootdir: /home/dl/projects/redhat/robottelo, 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-09-20 19:15:21 - conftest - DEBUG - Found WONTFIX in decorated tests ['1156555', '1269196', '1378009', '1245334', '1217635', '1226425', '1147100', '1311113', '1199150', '1204686', '1221971', '1103157', '1230902', '1230865', '1214312', '1079482']

2017-09-20 19:15:21 - conftest - DEBUG - Collected 1 test cases


tests/foreman/cli/test_host.py::HostCreateTestCase::test_positive_register_twice_with_uppercase_chars_in_hostname PASSED

================================================== 0 tests deselected ==================================================
============================================== 1 passed in 285.09 seconds ==============================================

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

name_chars[i] = name_chars[i].upper()
else:
name_chars[i] = name_chars[i].lower()
target_image = ''.join(name_chars)
Copy link
Contributor

Choose a reason for hiding this comment

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

One-linear suggestion instead all these lines with conditions and double conversion:

''.join((str.upper,str.lower)[i%2](ch) for i,ch in enumerate(name))

But maybe it's a bit too tricky, i'm not sure. Anyhow, not a blocker 🙂

Copy link
Contributor

Choose a reason for hiding this comment

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

Or at least you could drop else::

name_chars = list(name.lower())
for i in range(len(name_chars)):
    if i % 2 == 0:
        name_chars[i] = name_chars[i].upper()
target_image = ''.join(name_chars)

@ldjebran
Copy link
Contributor Author

@abalakh @oshtaier all comments addressed

generated host name was : YvWcSbBrWk

(sat-6.3.0) dlezz@elysion:~/projects/robottelo-fork$ pytest -v tests/foreman/cli/test_host.py::HostCreateTestCase::test_positive_register_twice_with_uppercase_chars_in_hostname
================================================= 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-09-25 11:33:16 - conftest - DEBUG - Deselect of WONTFIX BZs is disabled in settings


tests/foreman/cli/test_host.py::HostCreateTestCase::test_positive_register_twice_with_uppercase_chars_in_hostname PASSED

============================================== 1 passed in 303.77 seconds ==============================================

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

@oshtaier oshtaier merged commit 70f19f8 into SatelliteQE:master Sep 26, 2017
@ldjebran ldjebran deleted the qe_test_coverage_1361309 branch January 23, 2018 13:12
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

3 participants