bug#25025: python-shell-calculate-command is wrong

[Noam Postavsky]

On Wed, Aug 16, 2017 at 10:32 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> Could you please elaborate about the reason(s) for the breakage of the
> test?  I'm not sure I follow, and so cannot make up my mind whether I
> agree with removing the test.
>
> Thanks.

Certainly; this is the test:

(ert-deftest python-shell-calculate-command-1 ()
  "Check the command to execute is calculated correctly.
Using `python-shell-interpreter' and
`python-shell-interpreter-args'."
  (skip-unless (executable-find python-tests-shell-interpreter))
  (let ((python-shell-interpreter (executable-find
                                   python-tests-shell-interpreter))
        (python-shell-interpreter-args "-B"))
    (should (string=
             (format "%s %s"
                     (shell-quote-argument python-shell-interpreter)
                     python-shell-interpreter-args)
             (python-shell-calculate-command)))))

This is the old version of python-shell-calculate-command:

(defun python-shell-calculate-command ()
  "Calculate the string used to execute the inferior Python process."
  (format "%s %s"
          (shell-quote-argument python-shell-interpreter)
          python-shell-interpreter-args))

As you can see, the test just repeats the function body and checks for
equality. The new version of python-shell-calculate-command is

(defun python-shell-calculate-command ()
  "Calculate the string used to execute the inferior Python process."
  (format "%s %s"
          ;; `python-shell-make-comint' expects to be able to
          ;; `split-string-and-unquote' the result of this function.
          (combine-and-quote-strings (list python-shell-interpreter))
          python-shell-interpreter-args))

Which is no longer the same code as what is in the test. On Unixish
hosts it passes because the default value of
'python-shell-interpreter' "python" doesn't need any escaping to be
shell-quoted. combine-and-quote-strings likewise doesn't do anything
to arguments without spaces, so it gives the same thing. But on w32
shell-quote-argument always puts double quotes around its argument,
causing the test to fail.

We could update the test to use combine-and-quote-strings too, but
just having a second copy of the implementation doesn't seem like a
useful test to me.