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] host GET/PUT read content_source_id (BZ1488130) #5307

Merged

Conversation

ldjebran
Copy link
Contributor

@ldjebran ldjebran commented Sep 19, 2017

cover:

even if skipped 1488130
showing that closed

bz_bug_is_open(1488130)
2017-09-19 16:52:33 - robozilla.decorators - INFO - Bugzilla bug 1488130 not in cache. Fetching.
Out[5]: False

but locally the tests failed to skip, and failed on asserting that content_source_id in response or in read data

tests/foreman/api/test_host.py:1338: in test_positive_update_content_source_id
    self.assertIn('content_source_id', response_attributes)
E   AssertionError: 'content_source_id' not found in {u'comment': None, u'build_status': 0, u'environment_id': 5, u'managed': True, u'all_puppetclasses': [], u'model_id': None, u'ip': None,
...

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

codecov bot commented Sep 19, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5307   +/-   ##
=======================================
  Coverage   60.85%   60.85%           
=======================================
  Files          34       34           
  Lines        3671     3671           
=======================================
  Hits         2234     2234           
  Misses       1437     1437
Impacted Files Coverage Δ
robottelo/decorators/func_locker.py 93.23% <0%> (ø) ⬆️

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 00843ea...da1da8e. Read the comment docs.

'url': 'https://{0}:9090'.format(settings.server.hostname)})[0]
host = entities.Host().create()
# Update the host with content_source_id
response = client.put(
Copy link
Contributor

Choose a reason for hiding this comment

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

if you test read functionality in that test case, why don't you use usual host.upate() method?

Copy link
Contributor Author

@ldjebran ldjebran Sep 20, 2017

Choose a reason for hiding this comment

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

nailgun has no content_source field for the moment, and this is the direct call, we cannot add this field in nailgun as not returned at all for the moment, an will break host entity

Copy link
Contributor

Choose a reason for hiding this comment

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

but it can be passed as a parameter for PUT. Does update method fail on read operation?

Copy link
Contributor Author

@ldjebran ldjebran Sep 20, 2017

Choose a reason for hiding this comment

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

@oshtaier it can be passed as parameter in HTTP POST/PUT but that parameter is never returned back to be read
if you update the host entity with content_source field

class Host(...):
    def __init__(self, server_config=None, **kwargs):
        self._fields = {
            ...
            'content_source': entity_fields.OneToOneField(SmartProxy),

and after you want to create a host

host = entities.Host().create()
raceback (most recent call last):
  File "/home/dl/.pyenv/versions/sat-6.3/lib/python2.7/site-packages/IPython/core/interactiveshell.py", line 2882, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-8-933a92e5f845>", line 1, in <module>
    host = entities.Host(content_source=proxy).create()
  File "/home/dl/.pyenv/versions/2.7.13/envs/sat-6.3/lib/python2.7/site-packages/blinker_herald/base.py", line 137, in wrapper
    result = fn(*args, **kwargs)
  File "/home/dl/.pyenv/versions/2.7.13/envs/sat-6.3/lib/python2.7/site-packages/nailgun/entities.py", line 3183, in create
    id=self.create_json(create_missing)['id'],
  File "/home/dl/.pyenv/versions/2.7.13/envs/sat-6.3/lib/python2.7/site-packages/nailgun/entities.py", line 3272, in read
    result = super(Host, self).read(entity, attrs, ignore, params)
  File "/home/dl/.pyenv/versions/2.7.13/envs/sat-6.3/lib/python2.7/site-packages/nailgun/entity_mixins.py", line 799, in read
    entity_id = _get_entity_id(field_name, attrs)
  File "/home/dl/.pyenv/versions/2.7.13/envs/sat-6.3/lib/python2.7/site-packages/nailgun/entity_mixins.py", line 256, in _get_entity_id
    .format(field_name, (field_name, field_name_id), attrs.keys())
MissingValueError: Cannot find a value for the "content_source" field. Searched for keys named ('content_source', 'content_source_id'), but available keys are [u'comment', u'build_status', u'environment_id', u'managed', u'all_puppetclasses', u'model_id', u'ip', u'provision_method', u'updated_at', u'realm_name', u'compute_resource_id', u'puppet_status', u'sp_mac', u'operatingsystem_name', u'openscap_proxy', u'id', u'openscap_proxy_id', u'disk', u'location_id', u'domain_id', u'build_status_label', u'organization_name', 'host_parameters_attributes', u'location_name', u'ptable_id', u'subnet_id', u'openscap_proxy_name', u'sp_name', u'environment_name', u'architecture_name', u'domain_name', u'capabilities', u'subnet6_id', u'image_id', u'global_status', u'architecture_id', u'build', u'hostgroup_title', u'subnet_name', u'sp_ip', u'model_name', u'puppet_proxy_id', u'global_status_label', u'owner_id', u'operatingsystem_id', u'sp_subnet_id', u'puppet_ca_proxy_name', u'config_groups', u'name', u'permissions', u'compute_profile_name', u'interfaces', u'certname', u'image_name', u'organization_id', u'image_file', u'mac', u'ip6', u'puppet_ca_proxy_id', u'puppetclasses', u'puppet_ca_proxy', u'last_report', u'puppet_proxy', u'medium_name', u'last_compile', u'uuid', u'pxe_loader', u'ptable_name', u'medium_id', u'compute_resource_name', u'created_at', u'enabled', u'hostgroup_id', u'subnet6_name', u'hostgroup_name', u'use_image', u'compute_profile_id', u'all_parameters', u'owner_type', u'realm_id', u'installed_at', u'puppet_proxy_name'].

so in nailgun the post passed but reading the entity failed and create function failed with in the read function because that field is (content_source_id) is not returned

we have only on way to update that field is to use direct HTTP PUT
to verify read use the nailgun read_json to see if the key is there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oshtaier responding to your question , any operation that use read will fail on read operation

Copy link
Contributor

Choose a reason for hiding this comment

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

nailgun has no content_source field for the moment, and this is the direct call, we cannot add this field in nailgun as not returned at all for the moment, an will break host entity

That's basically a reason why i postponed API part of the issue whilst covered other interfaces. And i'm still not sure whether it's a good idea to introduce such test with direct api calls which demands refactor, maybe it's better to cover it when the issue is on_qa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abalakh , this test should never be refactored even when that field appear , as it has to verify directly the return of PUT

Copy link
Contributor

Choose a reason for hiding this comment

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

@ldjebran and what's your concern? update() will verify the field is present in output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abalakh update will verify all the fields and fail if any does not exist even before it reach the verification of the current field, we want to verify only this Field if the test fail it should fail only for this field.

@ldjebran
Copy link
Contributor Author

Put do not merge, as some comments on bug https://bugzilla.redhat.com/show_bug.cgi?id=1488130#c12 need more investigations

@ldjebran ldjebran force-pushed the 6.3.0_qe_test_coverage_1488130 branch from 2ffd01c to da1da8e Compare September 29, 2017 11:22
@ldjebran
Copy link
Contributor Author

@oshtaier @abalakh updated this PR as per bug https://bugzilla.redhat.com/show_bug.cgi?id=1488130 investigations and updated informations.

======================================= 2 passed, 67 deselected in 89.25 seconds =======================================
(sat-6.3.0) dlezz@elysion:~/projects/robottelo-fork$ pytest tests/foreman/api/test_host.py::HostTestCase -v -k "content_source_id"
================================================= 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 69 items 
2017-09-29 14:18:35 - conftest - DEBUG - Deselect of WONTFIX BZs is disabled in settings


tests/foreman/api/test_host.py::HostTestCase::test_positive_read_content_source_id <- robottelo/decorators/__init__.py PASSED
tests/foreman/api/test_host.py::HostTestCase::test_positive_update_content_source_id <- robottelo/decorators/__init__.py PASSED

================================================= 67 tests deselected ==================================================
======================================= 2 passed, 67 deselected in 90.21 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

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

@abalakh abalakh merged commit cbb3950 into SatelliteQE:master Oct 9, 2017
rochacbruno pushed a commit to rochacbruno/robottelo that referenced this pull request Nov 7, 2017
@ldjebran ldjebran deleted the 6.3.0_qe_test_coverage_1488130 branch January 23, 2018 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Issues and PRs involving the API 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