SatelliteQE/robottelo — pull request #1611 by Ichimonji10 (2014-10-22 17:26:52+00:00)
Title: Simplify search code, possibly fix a failing test

A Jenkins test has been failing for the past 7 test runs. The test fails when
run against a RHEL7 server (for the past 7 runs), but succeeds when run against
a RHEL65 server. The failing test is:

tests.foreman.api.test_repository.py.RepositorySyncTestCase.test_redhat_sync_1

It fails with the following (trimmed down) traceback:

Traceback (most recent call last):
File "/usr/lib64/python2.7/unittest/case.py", line 369, in run
testMethod()
File "…/tests/foreman/api/test_repository.py", line 279, in test_redhat_sync_1
"6.3",
File "…/robottelo/api/utils.py", line 131, in enable_rhrepo_and_fetchid
return entities.Repository().fetch_repoid(name=repo, org_id=org_id)
File "…/robottelo/entities.py", line 1957, in fetch_repoid
"The length of the results is:", len(results))
APIResponseError: ('The length of the results is:', 0)

I am unable to reproduce the failing behaviour when running the test locally. I
have run the test against both RHEL65 and RHEL7 machines. Of minor note is that
I had to fill in the `manifest` settings in `robottelo.properties` before
continuing with the test.

The first item on the stack is this statement from
`tests/foreman/api/test_repository.py`:

repo_id = utils.enable_rhrepo_and_fetchid(
"x86_64",
org_id,
"Red Hat Enterprise Linux Server",
repo,
"Red Hat Enterprise Linux 6 Server - RH Common (RPMs)",
"6.3",
)

The `repo` variable is treated as a constant:

repo = "Red Hat Enterprise Linux 6 Server - RH Common RPMs x86_64 6.3"

The second item on the stack is this statement from `robottelo/api/utils.py`:

return entities.Repository().fetch_repoid(name=repo, org_id=org_id)

The `repo` variable is identical to the `repo` variable from the first item in
the stack. The value of the `org_id` variable is determined while the parent
stack item is executing. An organization is created, and its ID is passed down.

The third and top item on the stack is this statement from
`robottelo/entities.py`:

if len(results) != 1:
raise APIResponseError(
"The length of the results is:", len(results))

What is `results`? This code sets `results`:

response = client.get(
self.path(which=None),
auth=get_server_credentials(),
verify=False,
data={u'organization_id': org_id,
u'search': 'name={}'.format(escape_search(name))}
)
response.raise_for_status()
results = response.json()['results']

This code lies within the `Repository` class, so `self.path(which=None)`
evaluates to a value such as this:

https://example.com/katello/api/v2/repositories

Thus, the root cause of this failure is that we're searching for a repository
that has a particular `organization_id` and `name`, and we're turning up zero
results.

Checking the format of the search above against the API documentation shows
discrepancies. Yes, the API documentation does state that a "search" key may be
submitted as part of a GET request. However, there are two things that make this
suspect:

* It is unknown how to use the "search" key.
* A "name" key may be submitted along with the search.

With these two points in mind, I assert that the following code is both simpler
and still correct:

response = client.get(
self.path(which=None),
auth=get_server_credentials(),
data={u'organization_id': org_id, u'name': name},
verify=False,
)

An empirical test shows that the test still succeeds when run against a RHEL 65
server. The test fails when run against a RHEL7 server, but this is likely for
other reasons. The unchanged test also fails horribly when run against that
machine. Something about dynflow.

Also of note is the `escape_search` method. This is the source of the method:

def escape_search(term):
"""Wraps a search term in " and escape term's " and \\ characters"""
strip_term = term.strip()
return u'"%s"' % strip_term.replace('\\', '\\\\').replace('"', '\\"')

The method is used in several UI tests, but not any CLI or API code. (That
brings up a question: why is it in module `robottelo.common.helpers`, given that
it only appears to be useful for UI tests? Also, doesn't something in the
standard library do this already? And more reliably, too?) Above, the method was
applied to the `name` variable. That caused this string:

Red Hat Enterprise Linux 6 Server - RH Common RPMs x86_64 6.3

To be transformed to this string:

"Red Hat Enterprise Linux 6 Server - RH Common RPMs x86_64 6.3"

See the difference? _Quotes are part of the search string._ That's not good, and
when combined with the usage of the "search" key in the GET request, that may be
the cause of the test failure. Why? One possible explanation is this: RHEL65 and
RHEL7 have different sets of libraries available. It is possible that the search
parsing libraries on these two systems work slightly differently, and whereas
RHEL 65 discards the outermost set of quotes, RHEL7 correctly keeps them.

That's just a possible explanation. Regardless, this change to code appears to
be a good one, even if it does not cause the failing test to start succeeding
again. The API documentation is "gold", and I want to use the simplest and most
straightforward search available as per the API documentation.