[test_display_callback.py] Use ANSIBLE_PYTHON_INTERPRETER var

[NickLaMuro]

Fixes test issues when using newer versions of ansible that have this patch:

ansible/ansible#50163

Since there are tests that end up triggering this warning and mess with the event locations that are returned.

Made a quick refactor to the fixture in this test file as well to break out the default env vars so it was a bit more DRY and re-usable. Related, and note worthy, I worked on these fixes while working on the following PR:

#335

And decided to make this an isolated patch instead of building of of that one (or vice versa). As a result, one of those PR is going to have merge conflicts once the other is merged, so I am prepared to rebase once that happens.

[ansible-zuul]

Build failed.

• ansible-tox-py36 : FAILURE in 9m 33s
• ansible-tox-py37 : FAILURE in 10m 52s
• ansible-build-python-tarball : SUCCESS in 3m 01s
• ansible-tox-linters : FAILURE in 5m 16s
• ansible-tox-py27 : FAILURE in 8m 29s

[AlanCoding]

I can, indeed, replicate the failure locally. I see how it was probably caused by the insertion of another warning, creating an unexpected verbose event.

I applied your patch and it seems to fix that particular issue, but the test still fails with

py.test test/integration/test_display_callback.py::test_callback_plugin_task_args_leak[playbook0]
================================================================================ test session starts ================================================================================
platform darwin -- Python 3.6.5, pytest-5.1.2, py-1.8.0, pluggy-0.12.0
rootdir: /Users/alancoding/Documents/repos/ansible-runner
collected 1 item                                                                                                                                                                    

test/integration/test_display_callback.py F                                                                                                                                   [100%]

===================================================================================== FAILURES ======================================================================================
__________________________________________________________________ test_callback_plugin_task_args_leak[playbook0] ___________________________________________________________________

executor = <ansible_runner.runner.Runner object at 0x10f90f128>
playbook = {'no_log_on_ok.yml': '\n- name: args should not be logged when no_log is set at the task or module level\n  connection... url=https://example.org username="PUBLIC" password="PRIVATE"\n    - copy: content="PRIVATE" dest="/tmp/tmp_no_log"\n'}
default_envvars = {'ANSIBLE_DEPRECATION_WARNINGS': 'False', 'ANSIBLE_PYTHON_INTERPRETER': '/Users/alancoding/.virtualenvs/runner/bin/python3.6'}

    @pytest.mark.parametrize('playbook', [
    {'no_log_on_ok.yml': '''
    - name: args should not be logged when no_log is set at the task or module level
      connection: local
      hosts: all
      gather_facts: no
      tasks:
        - shell: echo "PUBLIC"
        - shell: echo "PRIVATE"
          no_log: true
        - uri: url=https://example.org username="PUBLIC" password="PRIVATE"
        - copy: content="PRIVATE" dest="/tmp/tmp_no_log"
    '''},  # noqa
    ])
    def test_callback_plugin_task_args_leak(executor, playbook, default_envvars):
        executor.run()
        events = list(executor.events)
        assert events[0]['event'] == 'playbook_on_start'
        assert events[1]['event'] == 'playbook_on_play_start'
    
        # task 1
        assert events[2]['event'] == 'playbook_on_task_start'
        assert events[3]['event'] == 'runner_on_start'
        assert events[4]['event'] == 'runner_on_ok'
    
        # task 2 no_log=True
        assert events[5]['event'] == 'playbook_on_task_start'
        assert events[6]['event'] == 'runner_on_start'
        assert events[7]['event'] == 'runner_on_ok'
        assert 'PUBLIC' in json.dumps(events)
        assert 'PRIVATE' not in json.dumps(events)
        # make sure playbook was successful, so all tasks were hit
>       assert not events[-1]['event_data']['failures'], 'Unexpected playbook execution failure'
E       AssertionError: Unexpected playbook execution failure
E       assert not {'localhost': 1}

test/integration/test_display_callback.py:202: AssertionError
------------------------------------------------------------------------------- Captured stdout call --------------------------------------------------------------------------------

PLAY [args should not be logged when no_log is set at the task or module level] ***

TASK [shell] *******************************************************************
changed: [localhost]

TASK [shell] *******************************************************************
changed: [localhost]

TASK [uri] *********************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Unsupported parameters for (uri) module: username Supported parameters include: attributes, backup, body, body_format, client_cert, client_key, content, creates, delimiter, dest, directory_mode, follow, follow_redirects, force, force_basic_auth, group, headers, http_agent, method, mode, owner, regexp, remote_src, removes, return_content, selevel, serole, setype, seuser, src, status_code, timeout, unix_socket, unsafe_writes, url, url_password, url_username, use_proxy, validate_certs"}

PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=2    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

================================================================================= 1 failed in 2.98s =================================================================================

I applied an additional diff:

diff --git a/test/integration/test_display_callback.py b/test/integration/test_display_callback.py
index 9b651c9..ca130ad 100644
--- a/test/integration/test_display_callback.py
+++ b/test/integration/test_display_callback.py
@@ -177,7 +177,7 @@ def test_callback_plugin_no_log_filters(executor, playbook):
     - shell: echo "PUBLIC"
     - shell: echo "PRIVATE"
       no_log: true
-    - uri: url=https://example.org username="PUBLIC" password="PRIVATE"
+    - uri: url=https://example.org user="PUBLIC" password="PRIVATE"
     - copy: content="PRIVATE" dest="/tmp/tmp_no_log"
 '''},  # noqa
 ])

With your diff + this diff, the test passes for me locally.

I suspect this might related to a behavior change in Ansible core. The url_* parameters are preferred, user/password are aliases, so I would be happy to go with that if the test matrix for ansible runner still works. Ping @ryanpetrello in case he has any additional thoughts. The playbook involved is:

ansible-runner/test/integration/test_display_callback.py

Lines 162 to 171 in 39644c6

	- name: args should not be logged when no_log is set at the task or module level
	connection: local
	hosts: all
	gather_facts: no
	tasks:
	- shell: echo "PUBLIC"
	- shell: echo "PRIVATE"
	no_log: true
	- uri: url=https://example.org username="PUBLIC" password="PRIVATE"
	- copy: content="PRIVATE" dest="/tmp/tmp_no_log"

[NickLaMuro]

@AlanCoding Thanks for looking into this. This was originally opened to address some failures I was seeing in #335 locally, but both seem to have CI failures that I am not able to reproduce, so I appreciate the assist.

I will try make those changes today when time permits, but as we have already worked around this bug ourselves here:

ManageIQ/manageiq#19147

I hope you forgive me if I don't make this my first priority. 😅

[NickLaMuro]

Sorry for the delay on this, but it looks like I was getting the failures locally that are described here:

• pexpect.spawn() can hang when environment variables contain Unicode characters (or integers/floats) pexpect/pexpect#512
• improve error handling robustness for os.execvpe pexpect/ptyprocess#47

Which took me a while to track down. Didn't do anything as a result, but I now have a understanding of why that is happening better, and that I can pull a "Lando" and say "It's not my fault!".

---

Regarding the issue @AlanCoding mentioned, I wasn't able to get that failure to replicate on my machine, but I did rebase in the suggested fix of username -> user anyway since it does seem to happen in CI, and is consistent with how a different playbook in the same test is set up:

ansible-runner/test/integration/test_display_callback.py

Lines 292 to 305 in e1f0d86

	@pytest.mark.parametrize('playbook', [
	{'no_log_module_with_var.yml': '''
	- name: ensure that module-level secrets are redacted
	connection: local
	hosts: all
	vars:
	- pw: SENSITIVE
	tasks:
	- uri:
	url: https://example.org
	user: john-jacob-jingleheimer-schmidt
	password: "{{ pw }}"
	'''}, # noqa
	])

So figured it wouldn't hurt.

[ansible-zuul]

Build failed.

• ansible-tox-py36 : FAILURE in 9m 05s
• ansible-tox-py37 : FAILURE in 9m 33s
• ansible-build-python-tarball : SUCCESS in 2m 54s
• ansible-tox-linters : FAILURE in 5m 43s
• ansible-tox-py27 : FAILURE in 9m 31s

[ansible-zuul]

Build failed.

• ansible-tox-py36 : FAILURE in 9m 49s
• ansible-tox-py37 : FAILURE in 9m 57s
• ansible-build-python-tarball : SUCCESS in 2m 58s
• ansible-tox-linters : FAILURE in 5m 22s
• ansible-tox-py27 : FAILURE in 11m 07s

[AlanCoding]

I don't think the fixture default_envvars is needed for this test? Might be good to clean that up.

[NickLaMuro]

Yeah, still working out the ins and outs of pytest, so I think this was something I tried while trying to get things working. It works locally for me without it, so I will push up that change.

[NickLaMuro]

Current failures on CI:

fatal: [localhost]: FAILED! => {"changed": false, "checksum": "b0b7ba4666a92774e0d1afe02f2e20982b307a35", "msg": "Aborting, target uses selinux but python bindings (libselinux-python) aren't installed!"}

Seem like they are a build server configuration issue, correct?

[ansible-zuul]

Build failed.

• ansible-tox-py36 : FAILURE in 13m 09s
• ansible-tox-py37 : FAILURE in 9m 00s
• ansible-build-python-tarball : SUCCESS in 2m 58s
• ansible-tox-linters : FAILURE in 6m 02s
• ansible-tox-py27 : FAILURE in 12m 47s

[ryanpetrello]

@NickLaMuro any chance you're running w/ an environment variable set to an integer or float?

I found another way to reproduce that expect bug recently, and wonder if that's what you've encountered:

pexpect/ptyprocess#47 (comment)

[NickLaMuro]

@ryanpetrello Sorry for missing this a few days ago, but I will check. That said, am seeing it in CI as well, so if that is the case does that mean that CI would also have that issue?

[NickLaMuro]

@ryanpetrello After testing this while doing other things today, I wasn't able to re-create the errors locally again... sigh...

And this was testing all versions on both a VM (fedora-29) and my host OS (OSX), so unsure what I was doing a week ago that caused this to fail...

Regardless, I just did a push that fixed some flake8 errors that I had, so we will see what the build does this time. If it is what I reported previously, then probably is something on CI, but that might also be solved since that error wasn't present in this PR:

#335

Oh well... ¯\(°_o)/¯

[ansible-zuul]

Build failed.

• ansible-tox-py36 : FAILURE in 11m 32s
• ansible-tox-py37 : FAILURE in 11m 25s
• ansible-build-python-tarball : SUCCESS in 4m 35s
• ansible-tox-linters : SUCCESS in 6m 27s
• ansible-tox-py27 : FAILURE in 9m 23s

[AlanCoding]

I'm relatively sure the stuff in this was covered by other PRs. If you manage to still hit failures that shouldn't be happening, we can return to it.