The is_failed property of failed result of some ansible modules is False

[wangxin]

I have a simple script with content like below:

test_failed.py:

import pprint

def test_failed1(localhost):
    result = localhost.copy(src='not_exist', dest='/tmp/not_exist')

    pprint.pprint('is_failed={}'.format(result['localhost'].is_failed))
    pprint.pprint(result['localhost'])

This script called the ansible copy module with some arguments that will make it fail.

$ pytest test_failed.py --capture no
================================== test session starts ==================================
platform linux2 -- Python 2.7.12, pytest-4.6.5, py-1.8.1, pluggy-0.13.1
ansible: 2.8.7
rootdir: /var/johnar
plugins: repeat-0.8.0, xdist-1.28.0, forked-1.1.3, ansible-2.2.2
collected 1 item                                                                        

test_failed.py Loading callback plugin unnamed of type old, v1.0 from /usr/local/lib/python2.7/dist-packages/pytest_ansible/module_dispatcher/v28.py
META: ran handlers
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: johnar
<localhost> EXEC /bin/sh -c 'echo ~johnar && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /var/johnar/.ansible/tmp/ansible-tmp-1592465619.4-38779564525538 `" && echo ansible-tmp-1592465619.4-38779564525538="` echo /var/johnar/.ansible/tmp/ansible-tmp-1592465619.4-38779564525538 `" ) && sleep 0'
looking for "not_exist" at "/var/johnar/files/not_exist"
looking for "not_exist" at "/var/johnar/not_exist"
looking for "not_exist" at "./files/not_exist"
looking for "not_exist" at "./not_exist"
'is_failed=False'
{'_ansible_no_log': False,
 'changed': False,
 'exception': u'Traceback (most recent call last):\n  File "/usr/local/lib/python2.7/dist-packages/ansible/plugins/action/copy.py", line 464, in run\n    source = self._find_needle(\'files\', source)\n  File "/usr/local/lib/python2.7/dist-packages/ansible/plugins/action/__init__.py", line 1168, in _find_needle\n    return self._loader.path_dwim_relative_stack(path_stack, dirname, needle)\n  File "/usr/local/lib/python2.7/dist-packages/ansible/parsing/dataloader.py", line 319, in path_dwim_relative_stack\n    raise AnsibleFileNotFound(file_name=source, paths=[to_text(p) for p in search])\nAnsibleFileNotFound: Could not find or access \'not_exist\'\nSearched in:\n\t/var/johnar/files/not_exist\n\t/var/johnar/not_exist\n\t./files/not_exist\n\t./not_exist on the Ansible Controller.\nIf you are using a module and expect the file to exist on the remote, see the remote_src option\n',
 'invocation': {'dest': u'/tmp/not_exist',
                'module_args': {'dest': u'/tmp/not_exist',
                                'src': u'not_exist'},
                'src': u'not_exist'},
 'msg': u"Could not find or access 'not_exist'\nSearched in:\n\t/var/johnar/files/not_exist\n\t/var/johnar/not_exist\n\t./files/not_exist\n\t./not_exist on the Ansible Controller.\nIf you are using a module and expect the file to exist on the remote, see the remote_src option"}
.

=============================== 1 passed in 0.88 seconds ================================

In the log, we can see that the module failed with exception. However, the is_failed property of the test result is False. This does not make sense.

The reason is that pytest-ansible uses ResultAccumulator as callback to get the ansible module execution result. However, when ansible calls the ResultAccumulator::v2_runner_on_failed callback function, the 'failed' and 'skipped' keys are removed from the task_result. Without this 'failed' key in the result dictionary, calling results.py::ModuleResult.is_failed will always get 'False' unless the result dictionary has key 'rc' and its value is none-zero.

Ansible called the callback defined in ResultAccumulator here:
https://github.com/ansible/ansible/blob/stable-2.8/lib/ansible/plugins/strategy/__init__.py#L523

Then before passing the task_result to the callback, ansible called the clean_copy() method of TaskResult:
https://github.com/ansible/ansible/blob/stable-2.8/lib/ansible/executor/task_queue_manager.py#L325

The clean_copy() method of TaskResult removed keys 'failed' and 'skipped' from the result._result dictionary:
https://github.com/ansible/ansible/blob/stable-2.8/lib/ansible/executor/task_result.py#L146

IMO, the ResultAccumulator.v2_runner_on_failed method should add back the 'failed' key to the result dictionary. And the v2_runner_on_ok method should not be same as the v2_runner_on_failed method.

[kdelee]

I think #48 addresses part of the problem here

I'd be happy to review any further fixes if you send a PR

[Ruchip16]

closing the issue as there is no comment further and #48 addresses the problem here