Backport #37083 for 2.5 - IncludedFile comparison fix for free strategy (#38981)

* Consider parent also when comparing IncludedFile (#37083)

* Consider parent also when comparing IncludedFile

* Add new tests for IncludedFile and convert to pytest

(cherry picked from commit cdb79b0e3a)

* Add changelog for #37083

changelogs/fragments/include_comparison.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- dynamic includes - Fix IncludedFile comparison for free strategy (https://github.com/ansible/ansible/pull/37083)

lib/ansible/playbook/included_file.py

@@ -46,7 +46,7 @@ class IncludedFile:
             self._hosts.append(host)
 
     def __eq__(self, other):
-        return other._filename == self._filename and other._args == self._args
+        return other._filename == self._filename and other._args == self._args and other._task._parent._uuid == self._task._parent._uuid
 
     def __repr__(self):
         return "%s (%s): %s" % (self._filename, self._args, self._hosts)

test/units/playbook/test_included_file.py

@@ -21,7 +21,8 @@ __metaclass__ = type
 
 import os
 
-from ansible.compat.tests import unittest
+import pytest
+
 from ansible.compat.tests.mock import MagicMock
 from units.mock.loader import DictDataLoader
 
@@ -32,70 +33,122 @@ from ansible.executor import task_result
 from ansible.playbook.included_file import IncludedFile
 
 
-class TestIncludedFile(unittest.TestCase):
+@pytest.fixture
-    def test(self):
+def mock_iterator():
-        filename = 'somefile.yml'
+    mock_iterator = MagicMock(name='MockIterator')
+    mock_iterator._play = MagicMock(name='MockPlay')
+    return mock_iterator
+
+
+@pytest.fixture
+def mock_variable_manager():
+    # TODO: can we use a real VariableManager?
+    mock_variable_manager = MagicMock(name='MockVariableManager')
+    mock_variable_manager.get_vars.return_value = dict()
+    return mock_variable_manager
+
+
+def test_included_file_instantiation():
+    filename = 'somefile.yml'
+
+    inc_file = IncludedFile(filename=filename, args=[], task=None)
+
+    assert isinstance(inc_file, IncludedFile)
+    assert inc_file._filename == filename
+    assert inc_file._args == []
+    assert inc_file._task is None
+
+
+def test_process_include_results(mock_iterator, mock_variable_manager):
+    hostname = "testhost1"
+    hostname2 = "testhost2"
+
+    parent_task_ds = {'debug': 'msg=foo'}
+    parent_task = Task.load(parent_task_ds)
+
+    task_ds = {'include': 'include_test.yml'}
+    loaded_task = TaskInclude.load(task_ds, task_include=parent_task)
+
+    return_data = {'include': 'include_test.yml'}
+    # The task in the TaskResult has to be a TaskInclude so it has a .static attr
+    result1 = task_result.TaskResult(host=hostname, task=loaded_task, return_data=return_data)
+    result2 = task_result.TaskResult(host=hostname2, task=loaded_task, return_data=return_data)
+    results = [result1, result2]
+
+    fake_loader = DictDataLoader({'include_test.yml': ""})
+
+    res = IncludedFile.process_include_results(results, mock_iterator, fake_loader, mock_variable_manager)
+    assert isinstance(res, list)
+    assert len(res) == 1
+    assert res[0]._filename == os.path.join(os.getcwd(), 'include_test.yml')
+    assert res[0]._hosts == ['testhost1', 'testhost2']
+    assert res[0]._args == {}
+
 
-        inc_file = IncludedFile(filename=filename, args=[], task=None)
+def test_process_include_diff_files(mock_iterator, mock_variable_manager):
+    hostname = "testhost1"
+    hostname2 = "testhost2"
 
-        self.assertIsInstance(inc_file, IncludedFile)
+    parent_task_ds = {'debug': 'msg=foo'}
-        self.assertEquals(inc_file._filename, filename)
+    parent_task = Task.load(parent_task_ds)
-        self.assertEquals(inc_file._args, [])
-        self.assertEquals(inc_file._task, None)
 
-    def test_process_include_results(self):
+    task_ds = {'include': 'include_test.yml'}
-        hostname = "testhost1"
+    loaded_task = TaskInclude.load(task_ds, task_include=parent_task)
-        hostname2 = "testhost2"
 
-        parent_task_ds = {'debug': 'msg=foo'}
+    child_task_ds = {'include': 'other_include_test.yml'}
-        parent_task = Task()
+    loaded_child_task = TaskInclude.load(child_task_ds, task_include=loaded_task)
-        parent_task.load(parent_task_ds)
 
-        task_ds = {'include': 'include_test.yml'}
+    return_data = {'include': 'include_test.yml'}
-        task_include = TaskInclude()
+    # The task in the TaskResult has to be a TaskInclude so it has a .static attr
-        loaded_task = task_include.load(task_ds, task_include=parent_task)
+    result1 = task_result.TaskResult(host=hostname, task=loaded_task, return_data=return_data)
 
-        child_task_ds = {'include': 'other_include_test.yml'}
+    return_data = {'include': 'other_include_test.yml'}
-        child_task_include = TaskInclude()
+    result2 = task_result.TaskResult(host=hostname2, task=loaded_child_task, return_data=return_data)
-        loaded_child_task = child_task_include.load(child_task_ds, task_include=loaded_task)
+    results = [result1, result2]
 
-        return_data = {'include': 'include_test.yml'}
+    fake_loader = DictDataLoader({'include_test.yml': "",
-        # The task in the TaskResult has to be a TaskInclude so it has a .static attr
+                                  'other_include_test.yml': ""})
-        result1 = task_result.TaskResult(host=hostname, task=loaded_task, return_data=return_data)
 
-        return_data = {'include': 'other_include_test.yml'}
+    res = IncludedFile.process_include_results(results, mock_iterator, fake_loader, mock_variable_manager)
-        result2 = task_result.TaskResult(host=hostname2, task=loaded_child_task, return_data=return_data)
+    assert isinstance(res, list)
-        results = [result1, result2]
+    assert res[0]._filename == os.path.join(os.getcwd(), 'include_test.yml')
+    assert res[1]._filename == os.path.join(os.getcwd(), 'other_include_test.yml')
 
-        fake_loader = DictDataLoader({'include_test.yml': "",
+    assert res[0]._hosts == ['testhost1']
-                                      'other_include_test.yml': ""})
+    assert res[1]._hosts == ['testhost2']
 
-        mock_tqm = MagicMock(name='MockTaskQueueManager')
+    assert res[0]._args == {}
+    assert res[1]._args == {}
 
-        mock_play = MagicMock(name='MockPlay')
 
-        mock_iterator = MagicMock(name='MockIterator')
+def test_process_include_simulate_free(mock_iterator, mock_variable_manager):
-        mock_iterator._play = mock_play
+    hostname = "testhost1"
+    hostname2 = "testhost2"
 
-        mock_inventory = MagicMock(name='MockInventory')
+    parent_task_ds = {'debug': 'msg=foo'}
-        mock_inventory._hosts_cache = dict()
+    parent_task1 = Task.load(parent_task_ds)
+    parent_task2 = Task.load(parent_task_ds)
 
-        def _get_host(host_name):
+    task_ds = {'include': 'include_test.yml'}
-            return None
+    loaded_task1 = TaskInclude.load(task_ds, task_include=parent_task1)
+    loaded_task2 = TaskInclude.load(task_ds, task_include=parent_task2)
 
-        mock_inventory.get_host.side_effect = _get_host
+    return_data = {'include': 'include_test.yml'}
+    # The task in the TaskResult has to be a TaskInclude so it has a .static attr
+    result1 = task_result.TaskResult(host=hostname, task=loaded_task1, return_data=return_data)
+    result2 = task_result.TaskResult(host=hostname2, task=loaded_task2, return_data=return_data)
+    results = [result1, result2]
 
-        # TODO: can we use a real VariableManager?
+    fake_loader = DictDataLoader({'include_test.yml': ""})
-        mock_variable_manager = MagicMock(name='MockVariableManager')
-        mock_variable_manager.get_vars.return_value = dict()
 
-        res = IncludedFile.process_include_results(results, mock_iterator, fake_loader, mock_variable_manager)
+    res = IncludedFile.process_include_results(results, mock_iterator, fake_loader, mock_variable_manager)
-        self.assertIsInstance(res, list)
+    assert isinstance(res, list)
-        self.assertEquals(res[0]._filename, os.path.join(os.getcwd(), 'include_test.yml'))
+    assert len(res) == 2
-        self.assertEquals(res[1]._filename, os.path.join(os.getcwd(), 'other_include_test.yml'))
+    assert res[0]._filename == os.path.join(os.getcwd(), 'include_test.yml')
+    assert res[1]._filename == os.path.join(os.getcwd(), 'include_test.yml')
 
-        self.assertEquals(res[0]._hosts, ['testhost1'])
+    assert res[0]._hosts == ['testhost1']
-        self.assertEquals(res[1]._hosts, ['testhost2'])
+    assert res[1]._hosts == ['testhost2']
 
-        self.assertEquals(res[0]._args, {})
+    assert res[0]._args == {}
-        self.assertEquals(res[1]._args, {})
+    assert res[1]._args == {}