pause - ensure control characters are always set appropriately (#74568)

* pause - ensure control characters are always set appropriately

On some systems, curses.tigetstr() returns None, which does not work as a control character.

* Add unit tests

* Sort imports

* Skip on older Python

This is an action plugin and only runs on the controller, so no need to test of Python 2. Making
the import hackery work on Python 2 would required some more work which I am not sure is
worth it since we are moving away from Python 2 support on the controller.

* Make the tests work on Python 2 and 3

changelogs/fragments/73264-pause-emacs.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - pause - ensure control characters are always set to an appropriate value (https://github.com/ansible/ansible/issues/73264)

lib/ansible/plugins/action/pause.py

@@ -51,12 +51,12 @@ try:
 except ImportError:
     HAS_CURSES = False
 
+MOVE_TO_BOL = b'\r'
+CLEAR_TO_EOL = b'\x1b[K'
 if HAS_CURSES:
-    MOVE_TO_BOL = curses.tigetstr('cr')
+    # curses.tigetstr() returns None in some circumstances
-    CLEAR_TO_EOL = curses.tigetstr('el')
+    MOVE_TO_BOL = curses.tigetstr('cr') or MOVE_TO_BOL
-else:
+    CLEAR_TO_EOL = curses.tigetstr('el') or CLEAR_TO_EOL
-    MOVE_TO_BOL = b'\r'
-    CLEAR_TO_EOL = b'\x1b[K'
 
 
 class AnsibleTimeoutExceeded(Exception):

test/units/plugins/action/test_pause.py

@@ -0,0 +1,89 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2021 Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+import curses
+import importlib
+import io
+import pytest
+import sys
+
+from ansible.plugins.action import pause  # noqa: F401
+from ansible.module_utils.six import PY2
+
+builtin_import = 'builtins.__import__'
+if PY2:
+    builtin_import = '__builtin__.__import__'
+
+
+def test_pause_curses_tigetstr_none(mocker, monkeypatch):
+    monkeypatch.delitem(sys.modules, 'ansible.plugins.action.pause')
+
+    dunder_import = __import__
+
+    def _import(*args, **kwargs):
+        if args[0] == 'curses':
+            mock_curses = mocker.Mock()
+            mock_curses.setupterm = mocker.Mock(return_value=True)
+            mock_curses.tigetstr = mocker.Mock(return_value=None)
+            return mock_curses
+        else:
+            return dunder_import(*args, **kwargs)
+
+    mocker.patch(builtin_import, _import)
+
+    mod = importlib.import_module('ansible.plugins.action.pause')
+
+    assert mod.HAS_CURSES is True
+    assert mod.MOVE_TO_BOL == b'\r'
+    assert mod.CLEAR_TO_EOL == b'\x1b[K'
+
+
+def test_pause_missing_curses(mocker, monkeypatch):
+    monkeypatch.delitem(sys.modules, 'ansible.plugins.action.pause')
+
+    dunder_import = __import__
+
+    def _import(*args, **kwargs):
+        if args[0] == 'curses':
+            raise ImportError
+        else:
+            return dunder_import(*args, **kwargs)
+
+    mocker.patch(builtin_import, _import)
+
+    mod = importlib.import_module('ansible.plugins.action.pause')
+
+    with pytest.raises(AttributeError):
+        mod.curses
+
+    assert mod.HAS_CURSES is False
+    assert mod.MOVE_TO_BOL == b'\r'
+    assert mod.CLEAR_TO_EOL == b'\x1b[K'
+
+
+@pytest.mark.parametrize('exc', (curses.error, TypeError, io.UnsupportedOperation))
+def test_pause_curses_setupterm_error(mocker, monkeypatch, exc):
+    monkeypatch.delitem(sys.modules, 'ansible.plugins.action.pause')
+
+    dunder_import = __import__
+
+    def _import(*args, **kwargs):
+        if args[0] == 'curses':
+            mock_curses = mocker.Mock()
+            mock_curses.setupterm = mocker.Mock(side_effect=exc)
+            mock_curses.error = curses.error
+            return mock_curses
+        else:
+            return dunder_import(*args, **kwargs)
+
+    mocker.patch(builtin_import, _import)
+
+    mod = importlib.import_module('ansible.plugins.action.pause')
+
+    assert mod.HAS_CURSES is False
+    assert mod.MOVE_TO_BOL == b'\r'
+    assert mod.CLEAR_TO_EOL == b'\x1b[K'