Merge pull request #529 from akinomyoga/fix-test_man

Author: scop

bash_completion

@@ -2369,8 +2369,12 @@ fi
 unset compat_dir i _blacklist_glob
 
 # source user completion file
+#
+# Remark: We explicitly check that $user_completion is not '/dev/null' since
+#   /dev/null may be a regular file in broken systems and can contain arbitrary
+#   garbages of suppressed command outputs.
 user_completion=${BASH_COMPLETION_USER_FILE:-~/.bash_completion}
-[[ ${BASH_SOURCE[0]} != "$user_completion" && -r $user_completion && -f $user_completion ]] &&
+[[ $user_completion != "${BASH_SOURCE[0]}" && $user_completion != /dev/null && -r $user_completion && -f $user_completion ]] &&
     . $user_completion
 unset user_completion
 

test/t/conftest.py

@@ -186,29 +186,55 @@ def partialize(
 def bash(request) -> pexpect.spawn:
 
     logfile = None
+    histfile = None
+    tmpdir = None
+    bash = None
+
     if os.environ.get("BASHCOMP_TEST_LOGFILE"):
         logfile = open(os.environ["BASHCOMP_TEST_LOGFILE"], "w")
     elif os.environ.get("CI"):
         logfile = sys.stdout
+
     testdir = os.path.abspath(
         os.path.join(os.path.dirname(__file__), os.pardir)
     )
-    env = os.environ.copy()
-    env.update(
-        dict(
-            SRCDIR=testdir,  # TODO needed at least by bashrc
-            SRCDIRABS=testdir,
-            PS1=PS1,
-            INPUTRC="%s/config/inputrc" % testdir,
-            TERM="dumb",
-            LC_COLLATE="C",  # to match Python's default locale unaware sort
-            HISTFILE="/dev/null",  # to leave user's history file alone
-        )
+
+    # Create an empty temporary file for HISTFILE.
+    #
+    # To prevent the tested Bash processes from writing to the user's
+    # history file or any other files, we prepare an empty temporary
+    # file for each test.
+    #
+    # - Note that HISTFILE=/dev/null may not work.  It results in the
+    #   removal of the device /dev/null and the creation of a regular
+    #   file at /dev/null when the number of commands reach
+    #   HISTFILESIZE by Bash-4.3's bug.  This causes the execution of
+    #   garbages through BASH_COMPLETION_USER_FILE=/dev/null.  - Note
+    #   also that "unset -v HISTFILE" in "test/config/bashrc" was not
+    #   adopted because "test/config/bashrc" is loaded after the
+    #   history is read from the history file.
+    #
+    histfile = tempfile.NamedTemporaryFile(
+        prefix="bash-completion-test_", delete=False
     )
 
-    tmpdir = None
-    bash = None
     try:
+        # release the file handle so that Bash can open the file.
+        histfile.close()
+
+        env = os.environ.copy()
+        env.update(
+            dict(
+                SRCDIR=testdir,  # TODO needed at least by bashrc
+                SRCDIRABS=testdir,
+                PS1=PS1,
+                INPUTRC="%s/config/inputrc" % testdir,
+                TERM="dumb",
+                LC_COLLATE="C",  # to match Python's default locale unaware sort
+                HISTFILE=histfile.name,
+            )
+        )
+
         marker = request.node.get_closest_marker("bashcomp")
 
         # Set up the current working directory
@@ -306,6 +332,11 @@ def bash(request) -> pexpect.spawn:
             bash.close()
         if tmpdir:
             tmpdir.cleanup()
+        if histfile:
+            try:
+                os.remove(histfile.name)
+            except OSError:
+                pass
         if logfile and logfile != sys.stdout:
             logfile.close()
 
@@ -399,13 +430,21 @@ def assert_bash_exec(
 
 
 class bash_env_saved:
+    counter: int = 0
+
     def __init__(self, bash: pexpect.spawn, sendintr: bool = False):
+        bash_env_saved.counter += 1
+        self.prefix: str = "_BASHCOMP_TEST%d" % bash_env_saved.counter
+
         self.bash = bash
-        self.cwd: Optional[str] = None
+        self.cwd_changed: bool = False
         self.saved_shopt: Dict[str, int] = {}
         self.saved_variables: Dict[str, int] = {}
         self.sendintr = sendintr
 
+        self.noexcept: bool = False
+        self.captured_error: Optional[Exception] = None
+
     def __enter__(self):
         return self
 
@@ -418,111 +457,158 @@ def __exit__(
         self._restore_env()
         return None
 
+    def _safe_sendintr(self):
+        try:
+            self.bash.sendintr()
+            self.bash.expect_exact(PS1)
+        except Exception as e:
+            if self.noexcept:
+                self.captured_error = e
+            else:
+                raise
+
+    def _safe_exec(self, cmd: str):
+        try:
+            self.bash.sendline(cmd)
+            self.bash.expect_exact(cmd)
+            self.bash.expect_exact("\r\n" + PS1)
+        except Exception as e:
+            if self.noexcept:
+                self._safe_sendintr()
+                self.captured_error = e
+            else:
+                raise
+
+    def _safe_assert(self, cmd: str):
+        try:
+            assert_bash_exec(self.bash, cmd, want_output=None)
+        except Exception as e:
+            if self.noexcept:
+                self._safe_sendintr()
+                self.captured_error = e
+            else:
+                raise
+
     def _copy_variable(self, src_var: str, dst_var: str):
-        assert_bash_exec(
-            self.bash,
+        self._safe_exec(
             "if [[ ${%s+set} ]]; then %s=${%s}; else unset -v %s; fi"
             % (src_var, dst_var, src_var, dst_var),
         )
 
     def _unset_variable(self, varname: str):
-        assert_bash_exec(self.bash, "unset -v %s" % varname)
+        self._safe_exec("unset -v %s" % varname)
 
     def _save_cwd(self):
-        if not self.cwd:
-            self.cwd = self.bash.cwd
+        if not self.cwd_changed:
+            self.cwd_changed = True
+            self._copy_variable("PWD", "%s_OLDPWD" % self.prefix)
 
     def _check_shopt(self, name: str):
-        assert_bash_exec(
-            self.bash,
-            '[[ $(shopt -p %s) == "${_BASHCOMP_TEST_NEWSHOPT_%s}" ]]'
-            % (name, name),
+        self._safe_assert(
+            '[[ $(shopt -p %s) == "${%s_NEWSHOPT_%s}" ]]'
+            % (name, self.prefix, name),
         )
 
     def _unprotect_shopt(self, name: str):
         if name not in self.saved_shopt:
             self.saved_shopt[name] = 1
-            assert_bash_exec(
-                self.bash,
-                "_BASHCOMP_TEST_OLDSHOPT_%s=$(shopt -p %s; true)"
-                % (name, name),
+            self._safe_exec(
+                "%s_OLDSHOPT_%s=$(shopt -p %s || true)"
+                % (self.prefix, name, name),
             )
         else:
             self._check_shopt(name)
 
     def _protect_shopt(self, name: str):
-        assert_bash_exec(
-            self.bash,
-            "_BASHCOMP_TEST_NEWSHOPT_%s=$(shopt -p %s; true)" % (name, name),
+        self._safe_exec(
+            "%s_NEWSHOPT_%s=$(shopt -p %s || true)"
+            % (self.prefix, name, name),
         )
 
     def _check_variable(self, varname: str):
-        assert_bash_exec(
-            self.bash,
-            '[[ ${%s-%s} == "${_BASHCOMP_TEST_NEWVAR_%s-%s}" ]]'
-            % (varname, MAGIC_MARK2, varname, MAGIC_MARK2),
-        )
+        try:
+            self._safe_assert(
+                '[[ ${%s-%s} == "${%s_NEWVAR_%s-%s}" ]]'
+                % (varname, MAGIC_MARK2, self.prefix, varname, MAGIC_MARK2),
+            )
+        except Exception:
+            self._copy_variable(
+                "%s_NEWVAR_%s" % (self.prefix, varname), varname
+            )
+            raise
+        else:
+            if self.noexcept and self.captured_error:
+                self._copy_variable(
+                    "%s_NEWVAR_%s" % (self.prefix, varname), varname
+                )
 
     def _unprotect_variable(self, varname: str):
         if varname not in self.saved_variables:
             self.saved_variables[varname] = 1
-            self._copy_variable(varname, "_BASHCOMP_TEST_OLDVAR_" + varname)
+            self._copy_variable(
+                varname, "%s_OLDVAR_%s" % (self.prefix, varname)
+            )
         else:
             self._check_variable(varname)
 
     def _protect_variable(self, varname: str):
-        self._copy_variable(varname, "_BASHCOMP_TEST_NEWVAR_" + varname)
+        self._copy_variable(varname, "%s_NEWVAR_%s" % (self.prefix, varname))
 
     def _restore_env(self):
+        self.noexcept = True
+
         if self.sendintr:
-            self.bash.sendintr()
-            self.bash.expect_exact(PS1)
+            self._safe_sendintr()
 
         # We first go back to the original directory before restoring
         # variables because "cd" affects "OLDPWD".
-        if self.cwd:
+        if self.cwd_changed:
             self._unprotect_variable("OLDPWD")
-            assert_bash_exec(
-                self.bash, "command cd -- %s" % shlex.quote(str(self.cwd))
-            )
+            self._safe_exec('command cd -- "$%s_OLDPWD"' % self.prefix)
             self._protect_variable("OLDPWD")
-            self.cwd = None
+            self._unset_variable("%s_OLDPWD" % self.prefix)
+            self.cwd_changed = False
 
         for name in self.saved_shopt:
             self._check_shopt(name)
-            assert_bash_exec(
-                self.bash, 'eval "$_BASHCOMP_TEST_OLDSHOPT_%s"' % name
-            )
-            self._unset_variable("_BASHCOMP_TEST_OLDSHOPT_" + name)
-            self._unset_variable("_BASHCOMP_TEST_NEWSHOPT_" + name)
+            self._safe_exec('eval "$%s_OLDSHOPT_%s"' % (self.prefix, name))
+            self._unset_variable("%s_OLDSHOPT_%s" % (self.prefix, name))
+            self._unset_variable("%s_NEWSHOPT_%s" % (self.prefix, name))
         self.saved_shopt = {}
 
         for varname in self.saved_variables:
             self._check_variable(varname)
-            self._copy_variable("_BASHCOMP_TEST_OLDVAR_" + varname, varname)
-            self._unset_variable("_BASHCOMP_TEST_OLDVAR_" + varname)
-            self._unset_variable("_BASHCOMP_TEST_NEWVAR_" + varname)
+            self._copy_variable(
+                "%s_OLDVAR_%s" % (self.prefix, varname), varname
+            )
+            self._unset_variable("%s_OLDVAR_%s" % (self.prefix, varname))
+            self._unset_variable("%s_NEWVAR_%s" % (self.prefix, varname))
         self.saved_variables = {}
 
+        self.noexcept = False
+        if self.captured_error:
+            raise self.captured_error
+
     def chdir(self, path: str):
         self._save_cwd()
+        self.cwd_changed = True
         self._unprotect_variable("OLDPWD")
-        assert_bash_exec(self.bash, "command cd -- %s" % shlex.quote(path))
+        self._safe_exec("command cd -- %s" % shlex.quote(path))
         self._protect_variable("OLDPWD")
 
     def shopt(self, name: str, value: bool):
         self._unprotect_shopt(name)
         if value:
-            assert_bash_exec(self.bash, "shopt -s %s" % name)
+            self._safe_exec("shopt -s %s" % name)
         else:
-            assert_bash_exec(self.bash, "shopt -u %s" % name)
+            self._safe_exec("shopt -u %s" % name)
         self._protect_shopt(name)
 
     def write_variable(self, varname: str, new_value: str, quote: bool = True):
         if quote:
             new_value = shlex.quote(new_value)
         self._unprotect_variable(varname)
-        assert_bash_exec(self.bash, "%s=%s" % (varname, new_value))
+        self._safe_exec("%s=%s" % (varname, new_value))
         self._protect_variable(varname)
 
     # TODO: We may restore the "export" attribute as well though it is
@@ -531,7 +617,7 @@ def write_env(self, envname: str, new_value: str, quote: bool = True):
         if quote:
             new_value = shlex.quote(new_value)
         self._unprotect_variable(envname)
-        assert_bash_exec(self.bash, "export %s=%s" % (envname, new_value))
+        self._safe_exec("export %s=%s" % (envname, new_value))
         self._protect_variable(envname)
 
 
@@ -559,7 +645,7 @@ def diff_env(before: List[str], after: List[str], ignore: str):
         if not re.search(r"^(---|\+\+\+|@@ )", x)
         # Ignore variables expected to change:
         and not re.search(
-            r"^[-+](_|PPID|BASH_REMATCH|_BASHCOMP_TEST_\w+)=",
+            r"^[-+](_|PPID|BASH_REMATCH|_BASHCOMP_TEST\w+)=",
             x,
             re.ASCII,
         )

test/t/test_man.py

@@ -4,6 +4,7 @@
     assert_bash_exec,
     assert_complete,
     bash_env_saved,
+    is_bash_type,
     prepare_fixture_dir,
 )
 
@@ -105,8 +106,9 @@ def test_8(self, completion):
     def test_9(self, bash, completion):
         assert self.assumed_present in completion
 
-    @pytest.mark.complete(require_cmd=True)
     def test_10(self, request, bash, colonpath):
+        if not is_bash_type(bash, "man"):
+            pytest.skip("Command not found")
         with bash_env_saved(bash) as bash_env:
             bash_env.write_env(
                 "MANPATH",