Repository: ansible/ansible-runner

Issue:
Remove runner tests from ansible-core, and re-implement +/- 1 core testing here
The runner tests in core are pinned to some really old runner version, and don't really assert much of use, except that a very broad shape of data meets expectations.

Remove these tests from core, and re-implement better testing, hopefully against core devel/milestone, stable, and stable-1 at a minimum.

Discussion:

Patch:
diff --git a/docs/execution_environments.rst b/docs/execution_environments.rst
index 86b869a..f27f5d3 100644
--- a/docs/execution_environments.rst
+++ b/docs/execution_environments.rst
@@ -5,7 +5,7 @@ Using Runner with Execution Environments
 
 .. note::
 
-  For an Execution Environments general technology overview and to learn how get started using it in a few easy steps, see the `Getting started with Execution Environments guide <https://docs.ansible.com/ansible/devel/getting_started_ee/index.html>`_.
+  For an Execution Environments general technology overview and to learn how get started using it in a few easy steps, see the `Getting started with Execution Environments guide <https://ansible.readthedocs.io/en/latest/getting_started_ee/index.html>`_.
 
 **Execution Environments** are meant to be a consistent, reproducible, portable,
 and shareable method to run Ansible Automation jobs in the exact same way on
diff --git a/src/ansible_runner/runner.py b/src/ansible_runner/runner.py
index 02abba8..1b6a103 100644
--- a/src/ansible_runner/runner.py
+++ b/src/ansible_runner/runner.py
@@ -297,10 +297,11 @@ class Runner:
                 child.logfile_read = stdout_handle
             except pexpect.exceptions.ExceptionPexpect as e:
                 child = collections.namedtuple(
-                    'MissingProcess', 'exitstatus isalive close'
+                    'MissingProcess', 'exitstatus isalive expect close'
                 )(
                     exitstatus=127,
                     isalive=lambda: False,
+                    expect=lambda *args, **kwargs: None,
                     close=lambda: None,
                 )
 
@@ -341,9 +342,17 @@ class Runner:
                     Runner.handle_termination(child.pid)
                     self.timed_out = True
 
+            # fix for https://github.com/ansible/ansible-runner/issues/1330
+            # Since we're (ab)using pexpect's logging callback as our source of stdout data, we need to pump the stream one last
+            # time, in case any new output was written by the child between the last return from expect and its termination. Ideally
+            # this would have an arbitrarily large timeout value as well, in case a ridiculous amount of data was written, but just
+            # invoking one last pump should cover the vast majority of real-world cases.
+            child.expect(pexpect.EOF, timeout=5)
+
+            # close the child to ensure no more output will be written before we close the stream interposers
+            child.close()
             stdout_handle.close()
             stderr_handle.close()
-            child.close()
             self.rc = child.exitstatus if not (self.timed_out or self.canceled) else 254
 
         if self.canceled:


Test patch:
diff --git a/test/fixtures/projects/pexpect_timeout_data_loss/project/pb.yml b/test/fixtures/projects/pexpect_timeout_data_loss/project/pb.yml
new file mode 100644
index 0000000..ee19ab0
--- /dev/null
+++ b/test/fixtures/projects/pexpect_timeout_data_loss/project/pb.yml
@@ -0,0 +1,9 @@
+# part of the regression test for https://github.com/ansible/ansible-runner/issues/1330
+
+- hosts: localhost
+  gather_facts: no
+  tasks:
+    # sleep significantly longer than the configured pexpect timeout; the cancel callback will inject
+    # additional delay before the next process status sampling interval that can cause further output to be lost;
+    # if all is well, we'll do another loop over the child output until it's all been consumed...
+    - raw: sleep 2
diff --git a/test/integration/conftest.py b/test/integration/conftest.py
index 559031a..f9953bf 100644
--- a/test/integration/conftest.py
+++ b/test/integration/conftest.py
@@ -125,3 +125,62 @@ def container_image(request, cli, tmp_path):  # pylint: disable=W0621
         [runtime, 'rmi', '-f', image_name],
         bare=True,
     )
+
+
+@pytest.fixture
+def container_image_devel(request, cli, tmp_path):  # pylint: disable=W0621
+    branch = request.getfixturevalue('branch')
+
+    DOCKERFILE = f"""
+FROM quay.io/centos/centos:stream9
+ARG WHEEL
+COPY $WHEEL /$WHEEL
+
+# Need python 3.11 minimum for devel
+RUN dnf install -y python3.11 python3.11-pip git
+RUN alternatives --install /usr/bin/python3 python3 /usr/bin/python3.11 0
+RUN python3 -m pip install /$WHEEL git+https://github.com/ansible/ansible@{branch}
+
+RUN mkdir -p /runner/{{env,inventory,project,artifacts}} /home/runner/.ansible/tmp
+RUN chmod -R 777 /runner /home/runner
+WORKDIR /runner
+ENV HOME=/home/runner
+CMD ["ansible-runner", "run", "/runner"]
+"""
+
+    try:
+        containerized = request.getfixturevalue('containerized')
+        if not containerized:
+            yield None
+            return
+    except Exception:
+        # Test func doesn't use containerized
+        pass
+
+    if (env_image_name := os.getenv('RUNNER_TEST_IMAGE_NAME')):
+        yield env_image_name
+        return
+
+    cli(
+        ['pyproject-build', '-w', '-o', str(tmp_path)],
+        cwd=here.parent.parent,
+        bare=True,
+    )
+
+    wheel = next(tmp_path.glob('*.whl'))  # pylint: disable=R1708
+
+    runtime = request.getfixturevalue('runtime')
+    dockerfile_path = tmp_path / 'Dockerfile'
+    dockerfile_path.write_text(DOCKERFILE)
+    random_string = ''.join(random.choice(ascii_lowercase) for i in range(10))
+    image_name = f'ansible-runner-{random_string}-event-test'
+
+    cli(
+        [runtime, 'build', '--build-arg', f'WHEEL={wheel.name}', '--rm=true', '-t', image_name, '-f', str(dockerfile_path), str(tmp_path)],
+        bare=True,
+    )
+    yield image_name
+    cli(
+        [runtime, 'rmi', '-f', image_name],
+        bare=True,
+    )
diff --git a/test/integration/test_core_integration.py b/test/integration/test_core_integration.py
new file mode 100644
index 0000000..f9ccb30
--- /dev/null
+++ b/test/integration/test_core_integration.py
@@ -0,0 +1,84 @@
+import sys
+import pytest
+
+from ansible_runner.interface import run
+
+
+TEST_BRANCHES = (
+    'devel',
+    'milestone',
+    'stable-2.16',   # current stable
+    'stable-2.15',   # stable - 1
+)
+
+
+@pytest.mark.test_all_runtimes
+@pytest.mark.parametrize('branch', TEST_BRANCHES)
+@pytest.mark.skipif(sys.platform == 'darwin', reason='does not work on macOS')
+def test_adhoc(tmp_path, runtime, branch, container_image_devel):  # pylint: disable=W0613
+    # pvt_data_dir is mounted on the container, so it must contain the expected directories
+    project_dir = tmp_path / 'project'
+    project_dir.mkdir()
+    r = run(private_data_dir=str(tmp_path),
+            host_pattern='localhost',
+            module='shell',
+            module_args='pwd',
+            process_isolation_executable=runtime,
+            process_isolation=True,
+            container_image=container_image_devel,
+            )
+
+    assert r.status == 'successful'
+    assert r.rc == 0
+    assert 'ok' in r.stats
+    assert 'localhost' in r.stats['ok']
+    events = [x['event'] for x in r.events if x['event'] != 'verbose']
+    assert len(events) == 4
+
+
+@pytest.mark.test_all_runtimes
+@pytest.mark.parametrize('branch', TEST_BRANCHES)
+@pytest.mark.skipif(sys.platform == 'darwin', reason='does not work on macOS')
+def test_playbook(tmp_path, runtime, branch, container_image_devel):  # pylint: disable=W0613
+    PLAYBOOK = """
+- hosts: localhost
+  gather_facts: False
+  tasks:
+    - set_fact:
+        foo: bar
+"""
+
+    # pvt_data_dir is mounted on the container, so it must contain the expected directories
+    project_dir = tmp_path / 'project'
+    project_dir.mkdir()
+    inventory_dir = tmp_path / 'inventory'
+    inventory_dir.mkdir()
+
+    hosts_file = inventory_dir / 'hosts'
+    hosts_file.write_text('localhost\n')
+
+    playbook = project_dir / 'test.yml'
+    playbook.write_text(PLAYBOOK)
+
+    r = run(private_data_dir=str(tmp_path),
+            playbook='test.yml',
+            process_isolation_executable=runtime,
+            process_isolation=True,
+            container_image=container_image_devel,
+            )
+
+    expected_events = [
+        'playbook_on_start',
+        'playbook_on_play_start',
+        'playbook_on_task_start',
+        'runner_on_start',
+        'runner_on_ok',
+        'playbook_on_stats',
+    ]
+
+    assert r.status == 'successful'
+    assert r.rc == 0
+    assert 'ok' in r.stats
+    assert 'localhost' in r.stats['ok']
+    events = [x['event'] for x in r.events if x['event'] != 'verbose']
+    assert events == expected_events
diff --git a/test/integration/test_runner.py b/test/integration/test_runner.py
index 553d947..e00112b 100644
--- a/test/integration/test_runner.py
+++ b/test/integration/test_runner.py
@@ -4,12 +4,13 @@ import json
 import os
 import re
 import sys
+import time
 
 from test.utils.common import iterate_timeout
 
 import pytest
 
-from ansible_runner import Runner
+from ansible_runner import Runner, run
 from ansible_runner.exceptions import AnsibleRunnerException
 
 
@@ -279,3 +280,16 @@ def test_set_extra_vars(rc):
         with open(os.path.join(rc.artifact_dir, 'stdout')) as f:
             if 'hello there' in f.read():
                 break
+
+
+# regression test for https://github.com/ansible/ansible-runner/issues/1330
+def test_pexpect_timeout(project_fixtures):
+    r = run(
+        private_data_dir=str(project_fixtures / 'pexpect_timeout_data_loss'),
+        playbook='pb.yml',
+        settings={"pexpect_timeout": 0.1},  # set the pexpect timeout very low
+        cancel_callback=lambda: time.sleep(3) or False,  # induce enough delay in the child polling loop that the child will exit before being polled again
+    )
+
+    # ensure we got playbook_on_stats; if pexpect ate it, we won't...
+    assert any(ev for ev in r.events if ev.get('event', None) == 'playbook_on_stats')