Add missing mock for locate_oc_binary method

[pilou-]

When locate_oc_binary has not been mocked, the test suite fails when oc executable is available.

[openshift-bot]

Can one of the admins verify this patch?
I understand the following commands:

• bot, add author to whitelist
• bot, test pull request
• bot, test pull request once

[openshift-bot]

Can one of the admins verify this patch?
I understand the following commands:

• bot, add author to whitelist
• bot, test pull request
• bot, test pull request once

[tbielawa]

Interesting.

roles/lib_openshift/src/test/unit/test_oc_adm_registry.py:253: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
roles/lib_openshift/library/oc_adm_registry.py:2555: in run_ansible
    ocregistry = Registry(rconfig, params['debug'])
roles/lib_openshift/library/oc_adm_registry.py:2236: in __init__
    self.version = OCVersion(registry_config.kubeconfig, verbose)
roles/lib_openshift/library/oc_adm_registry.py:2165: in __init__
    super(OCVersion, self).__init__(None, config)
roles/lib_openshift/library/oc_adm_registry.py:965: in __init__
    self.oc_binary = locate_oc_binary()
.tox/py27-unit/lib/python2.7/site-packages/mock/mock.py:1062: in __call__
    return _mock_self._mock_call(*args, **kwargs)
.tox/py27-unit/lib/python2.7/site-packages/mock/mock.py:1121: in _mock_call
    result = next(effect)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
obj = <listiterator object at 0x7f2055c7d990>, _next = <built-in function next>
    def next(obj, _next=_next):
        if getattr(obj, '__next__', None):
            return obj.__next__()
>       return _next(obj)
E       StopIteration
.tox/py27-unit/lib/python2.7/site-packages/mock/mock.py:109: StopIteration
========= 1 failed, 711 passed, 72 skipped, 1 xfailed in 22.55 seconds =========

We get the super(OCVersion, self).__init__(None, config) call which triggers self.oc_binary = locate_oc_binary()

That's mocked and is set up as having side effects. The locate_oc_binary() function must be called multiple times. As there is only 1 side effect set on the mock locater any additional calls to it would fail. Maybe instead of using side effects you set the return_value for that function instead. Since it'll be (we want it to be) the same every successive call.

[pilou-]

I didn't notice the method was called twice, thanks !

[tbielawa]

Please use return_value instead.

[tbielawa]

For these mock_oc_binarys I think it's safer to simply assign oc to their return_value property. Can you swap these to use return_value instead? That will save us from any pain in the future if the code base changes again and the number of calls changes. return_value will always return just that one value. side_effect on an iterator will run out of values (as we saw before).

    mock_oc_binary.return_value = 'oc'

side_effect is good at what it does, returning items from an iterable or anything from a function, but it's a little too much for this use-case.

[pilou-]

@tbielawa done (return_value is used).

[tbielawa]

aos-ci-test

[tbielawa]

[test]

[openshift-bot]

success: "aos-ci-jenkins/OS_3.5_NOT_containerized, aos-ci-jenkins/OS_3.5_NOT_containerized_e2e_tests" for 93a979a (logs)

[openshift-bot]

success: "aos-ci-jenkins/OS_3.5_containerized, aos-ci-jenkins/OS_3.5_containerized_e2e_tests" for 93a979a (logs)

[openshift-bot]

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for 93a979a (logs)

[openshift-bot]

success: "aos-ci-jenkins/OS_3.6_NOT_containerized, aos-ci-jenkins/OS_3.6_NOT_containerized_e2e_tests" for 93a979a (logs)

[tbielawa]

/me wipes the flakes off this one openshift/origin#13846

[tbielawa]

re[test]

[openshift-bot]

Evaluated for openshift ansible test up to 93a979a

[openshift-bot]

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_openshift_ansible/64/) (Base Commit: 307bc94)

[pilou-]

Same failure, meanwhile a fix for the failing test (#3966) was merged. Should I rebase this pull-request ?

[tbielawa]

@pilou- I am not sure if that would do anything. I am going to poke @sdodson here and ask for advice. I think in this mornings team meeting Scott was saying we're waiting for a repository to update, but I'm not sure if that would effect this, or if a change that @mtnbikenc is working on is required as well..

@sdodson @mtnbikenc please advise 😻

[sdodson]

should be fixed now
[merge]

[openshift-bot]

Evaluated for openshift ansible merge up to 93a979a

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_openshift_ansible/269/) (Base Commit: 662aac5)