Patchwork [1,of,3] debuginstall: convert unified test to pure unit test (issue4128)

login
register
mail settings
Submitter Chris Jerdonek
Date Dec. 28, 2013, 1:20 a.m.
Message ID <3a91ff8b2a83bcfbad00.1388193600@stonewall.local>
Download mbox | patch
Permalink /patch/3237/
State Superseded
Headers show

Comments

Chris Jerdonek - Dec. 28, 2013, 1:20 a.m.
# HG changeset patch
# User Chris Jerdonek <chris.jerdonek@gmail.com>
# Date 1388190507 28800
#      Fri Dec 27 16:28:27 2013 -0800
# Node ID 3a91ff8b2a83bcfbad000f0140c9df67b0022522
# Parent  4274eda143cb1025be1130ffdaaf62370a2a6961
debuginstall: convert unified test to pure unit test (issue4128)

This change converts the test file for debuginstall from a unified test to
a pure unit test.  This gives us more flexibility in testing the output
of "hg debuginstall".  In particular, this will let us test in a
straightforward way the changes to debuginstall for issue4128.  For example,
the change lets us use Python to calculate the expected value of substrings
of the output, and then compare them.
Matt Mackall - Dec. 30, 2013, 11:30 p.m.
On Fri, 2013-12-27 at 17:20 -0800, Chris Jerdonek wrote:
> # HG changeset patch
> # User Chris Jerdonek <chris.jerdonek@gmail.com>
> # Date 1388190507 28800
> #      Fri Dec 27 16:28:27 2013 -0800
> # Node ID 3a91ff8b2a83bcfbad000f0140c9df67b0022522
> # Parent  4274eda143cb1025be1130ffdaaf62370a2a6961
> debuginstall: convert unified test to pure unit test (issue4128)
> 
> This change converts the test file for debuginstall from a unified test to
> a pure unit test.  This gives us more flexibility in testing the output
> of "hg debuginstall".  In particular, this will let us test in a
> straightforward way the changes to debuginstall for issue4128.  For example,
> the change lets us use Python to calculate the expected value of substrings
> of the output, and then compare them.

Going from .t to .py with custom code to do line matching is two steps
backward, one step forward. I'd much rather you added optional line
support in .t output so the next person who needs this feature doesn't
have to rewrite their test in the bigger, harder-to-maintain .py test
style.

But I really don't see the reason to do this at all for this bug. I
guess your new output looks like this:

    $ hg debuginstall
    displaying install info...
 -->sys.executable: '/Users/chris/dev/.virtualenvs/default/bin/python'
    sys.version: 2.7.6 (default, Nov 12 2013, 13:26:39)
    [GCC 4.2.1 Compatible Apple Clang 4.1
((tags/Apple/clang-421.11.66))]

    checking encoding (UTF-8)...
    checking Python lib (/Users/chris/dev/.virtualenvs/default/lib/
      python2.7)...
    checking installed modules (/Users/chris/dev/mercurial)...
    checking templates (/Users/chris/dev/mercurial/templates)...
    checking commit editor...
    checking username...
    no problems detected

And the issue is that sys.version routinely contains newlines... so it
can show us some more trivia we're not terribly concerned about.
Consider making it look like this:

    $ hg debuginstall
    checking encoding (UTF-8)...
    checking Python executable (/Users/chris/dev/.virtualenvs/default/bin/python)...
    checking Python version (2.7.6)...
    checking Python lib (/Users/chris/dev/.virtualenvs/default/lib/
      python2.7)...
    checking installed modules (/Users/chris/dev/mercurial)...
    checking templates (/Users/chris/dev/mercurial/templates)...
    checking commit editor...
    checking username...
    no problems detected

Notice the uniformity. In the unlikely event that there's confusion or
concern about which compiler was used to build the Python Mercurial's
using.. you can always just run it using the displayed path.

While we're on the topic of debuginstall, it would be useful to check if
there are multiple copies of 'hg' (or sys.argv[0]) in the path, as users
of a certain fruit-themed operating system are extremely prone to
simultaneously installing different versions from different sources or
package managers:

    $ hg debuginstall
    checking hg path (/usr/bin/hg)...
     warning! multiple installs of hg detected:
      /usr/bin/hg
      /opt/bin/hg
      /some/crappy/pypi/thing/hg
      ...
Chris Jerdonek - Dec. 31, 2013, 12:34 a.m.
On Mon, Dec 30, 2013 at 3:30 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Fri, 2013-12-27 at 17:20 -0800, Chris Jerdonek wrote:
>> # HG changeset patch
>> # User Chris Jerdonek <chris.jerdonek@gmail.com>
>> # Date 1388190507 28800
>> #      Fri Dec 27 16:28:27 2013 -0800
>> # Node ID 3a91ff8b2a83bcfbad000f0140c9df67b0022522
>> # Parent  4274eda143cb1025be1130ffdaaf62370a2a6961
>> debuginstall: convert unified test to pure unit test (issue4128)
>>
>> This change converts the test file for debuginstall from a unified test to
>> a pure unit test.  This gives us more flexibility in testing the output
>> of "hg debuginstall".  In particular, this will let us test in a
>> straightforward way the changes to debuginstall for issue4128.  For example,
>> the change lets us use Python to calculate the expected value of substrings
>> of the output, and then compare them.
>
> Going from .t to .py with custom code to do line matching is two steps
> backward, one step forward. I'd much rather you added optional line
> support in .t output so the next person who needs this feature doesn't
> have to rewrite their test in the bigger, harder-to-maintain .py test
> style.
>
> But I really don't see the reason to do this at all for this bug.

The other testing feature I needed for this issue is the ability to
compute the expected value of a substring of the test output.

For example, another part of issue 4128 is to show "information about
the Python that Mercurial was compiled against."  So I would like
debuginstall to display something like the following for one of its
output lines--

  using C extensions (yes)...
or
  using C extensions (no)...

In a .py file I can compute whether that should be "yes" or "no."  Can
that be done using a .t file?  Any suggestions?  Another complication
is that it might make sense to show additional lines of output if the
answer is "yes."  Testing with a .py file doesn't impose constraints
on the form of the output.

By the way, the amount of custom code for line matching can also be
reduced by sharing code with tests/run-tests.py:

http://selenic.com/hg/file/4274eda143cb/tests/run-tests.py#l603

I will add to the tracker your suggestion about multiple copies of
'hg' being on the path.

Thanks,
--Chris

> I guess your new output looks like this:
>
>     $ hg debuginstall
>     displaying install info...
>  -->sys.executable: '/Users/chris/dev/.virtualenvs/default/bin/python'
>     sys.version: 2.7.6 (default, Nov 12 2013, 13:26:39)
>     [GCC 4.2.1 Compatible Apple Clang 4.1
> ((tags/Apple/clang-421.11.66))]
>
>     checking encoding (UTF-8)...
>     checking Python lib (/Users/chris/dev/.virtualenvs/default/lib/
>       python2.7)...
>     checking installed modules (/Users/chris/dev/mercurial)...
>     checking templates (/Users/chris/dev/mercurial/templates)...
>     checking commit editor...
>     checking username...
>     no problems detected
>
> And the issue is that sys.version routinely contains newlines... so it
> can show us some more trivia we're not terribly concerned about.
> Consider making it look like this:
>
>     $ hg debuginstall
>     checking encoding (UTF-8)...
>     checking Python executable (/Users/chris/dev/.virtualenvs/default/bin/python)...
>     checking Python version (2.7.6)...
>     checking Python lib (/Users/chris/dev/.virtualenvs/default/lib/
>       python2.7)...
>     checking installed modules (/Users/chris/dev/mercurial)...
>     checking templates (/Users/chris/dev/mercurial/templates)...
>     checking commit editor...
>     checking username...
>     no problems detected
>
> Notice the uniformity. In the unlikely event that there's confusion or
> concern about which compiler was used to build the Python Mercurial's
> using.. you can always just run it using the displayed path.
>
> While we're on the topic of debuginstall, it would be useful to check if
> there are multiple copies of 'hg' (or sys.argv[0]) in the path, as users
> of a certain fruit-themed operating system are extremely prone to
> simultaneously installing different versions from different sources or
> package managers:
>
>     $ hg debuginstall
>     checking hg path (/usr/bin/hg)...
>      warning! multiple installs of hg detected:
>       /usr/bin/hg
>       /opt/bin/hg
>       /some/crappy/pypi/thing/hg
>       ...
>
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
Matt Mackall - Dec. 31, 2013, 1:09 a.m.
On Mon, 2013-12-30 at 16:34 -0800, Chris Jerdonek wrote:
> On Mon, Dec 30, 2013 at 3:30 PM, Matt Mackall <mpm@selenic.com> wrote:
> > On Fri, 2013-12-27 at 17:20 -0800, Chris Jerdonek wrote:
> >> # HG changeset patch
> >> # User Chris Jerdonek <chris.jerdonek@gmail.com>
> >> # Date 1388190507 28800
> >> #      Fri Dec 27 16:28:27 2013 -0800
> >> # Node ID 3a91ff8b2a83bcfbad000f0140c9df67b0022522
> >> # Parent  4274eda143cb1025be1130ffdaaf62370a2a6961
> >> debuginstall: convert unified test to pure unit test (issue4128)
> >>
> >> This change converts the test file for debuginstall from a unified test to
> >> a pure unit test.  This gives us more flexibility in testing the output
> >> of "hg debuginstall".  In particular, this will let us test in a
> >> straightforward way the changes to debuginstall for issue4128.  For example,
> >> the change lets us use Python to calculate the expected value of substrings
> >> of the output, and then compare them.
> >
> > Going from .t to .py with custom code to do line matching is two steps
> > backward, one step forward. I'd much rather you added optional line
> > support in .t output so the next person who needs this feature doesn't
> > have to rewrite their test in the bigger, harder-to-maintain .py test
> > style.
> >
> > But I really don't see the reason to do this at all for this bug.
> 
> The other testing feature I needed for this issue is the ability to
> compute the expected value of a substring of the test output.
> 
> For example, another part of issue 4128 is to show "information about
> the Python that Mercurial was compiled against."  So I would like
> debuginstall to display something like the following for one of its
> output lines--
> 
>   using C extensions (yes)...
> or
>   using C extensions (no)...
> 
> In a .py file I can compute whether that should be "yes" or "no."  Can
> that be done using a .t file?

Yes, using conditionals. There is not yet a conditional for pure as
there's been no call for it to date. But use a wildcard and call it
done, please. At the end of the day, it's just debugging output.

>   Any suggestions?  Another complication
> is that it might make sense to show additional lines of output if the
> answer is "yes."  Testing with a .py file doesn't impose constraints
> on the form of the output.

It sure doesn't. Instead it imposes overhead on maintenance and
comprehension. Which is why there are so few .py tests and why I'm
pushing back on this.

Patch

diff --git a/tests/test-install.t b/tests/test-debuginstall.py
rename from tests/test-install.t
rename to tests/test-debuginstall.py
--- a/tests/test-install.t
+++ b/tests/test-debuginstall.py
@@ -1,22 +1,80 @@ 
-hg debuginstall
-  $ hg debuginstall
-  checking encoding (ascii)...
-  checking Python lib (*lib*)... (glob)
-  checking installed modules (*mercurial)... (glob)
-  checking templates (*mercurial?templates)... (glob)
-  checking commit editor...
-  checking username...
-  no problems detected
+import collections
+import fnmatch
+import os
+import subprocess
+import textwrap
+import unittest
 
-hg debuginstall with no username
-  $ HGUSER= hg debuginstall
-  checking encoding (ascii)...
-  checking Python lib (*lib*)... (glob)
-  checking installed modules (*mercurial)... (glob)
-  checking templates (*mercurial?templates)... (glob)
-  checking commit editor...
-  checking username...
-   no username supplied (see "hg help config")
-   (specify a username in your configuration file)
-  1 problems detected, please check your install!
-  [1]
+import silenttestrunner
+
+def calldebuginstall(env=None):
+    if env is not None:
+        new_env = os.environ.copy()
+        new_env.update(env)
+        env = new_env
+    cmd = "hg debuginstall"
+    p = subprocess.Popen(cmd, shell=True, env=env,
+                         stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    stdout, stderr = p.communicate()
+    return stdout, stderr, p.returncode
+
+def tolines(s):
+    return textwrap.dedent(s).splitlines()
+
+class testdebuginstall(unittest.TestCase):
+
+    def checklines(self, actual, expected, wildcard=False):
+        if wildcard:
+            match = fnmatch.fnmatchcase
+        else:
+            match = lambda s, t: s == t
+        for eline in expected:
+            aline = actual.popleft()
+            if not match(aline, eline):
+                msg = ("line mismatch (wildcard=%s):\n"
+                       "  actual:   %r\n"
+                       "  expected: %r\n"
+                       "remaining lines-->\n%s"%
+                       (wildcard, aline, eline, "\n".join(actual)))
+                raise AssertionError(msg)
+
+    def test_basic(self):
+        stdout, stderr, returncode = calldebuginstall()
+        lines = stdout.splitlines()
+        self.assertEqual(0, returncode)
+
+        # We use a deque so we can call popleft() and compare lines without
+        # keeping track of a list index.
+        lines = collections.deque(lines)
+        self.checklines(lines, ["checking encoding (ascii)..."])
+        expected = tolines("""\
+        checking Python lib (*lib*)...
+        checking installed modules (*mercurial)...
+        checking templates (*mercurial?templates)...
+        """)
+        self.checklines(lines, expected, wildcard=True)
+        expected = tolines("""\
+        checking commit editor...
+        checking username...
+        no problems detected
+        """)
+        self.checklines(lines, expected)
+        self.assertFalse(lines, msg=("there were extra lines-->\n%s" %
+                                     "\n".join(lines)))
+
+    def test_nousername(self):
+        stdout, err, returncode = calldebuginstall(env={'HGUSER': ''})
+        lines = stdout.splitlines()
+        self.assertEqual(1, returncode)
+
+        # Only check the username-specific output.
+        expected = tolines("""\
+        checking username...
+         no username supplied (see "hg help config")
+         (specify a username in your configuration file)
+        1 problems detected, please check your install!
+        """)
+        self.assertEqual(lines[-4:], expected)
+
+if __name__ == '__main__':
+    silenttestrunner.main(__name__)