[PATCH 2 of 2 V2] tests/run-tests: use hghave.py

[Mads Kiilerich]

Adrian Buehlmann wrote, On 06/14/2012 03:48 PM:
> # HG changeset patch
> # User Adrian Buehlmann <adrian at cadifra.com>
> # Date 1339677848 -7200
> # Node ID e08ba77235ac9abff2ab821b257e42abffa59553
> # Parent  b92c1e526b98c23de4f2fc4e16303e8171b8b861
> tests/run-tests: use hghave.py
>
> Eliminates the start of a subprocess for #if requirements checking.

I pushed patch 1 to crew, thanks.

But

> diff --git a/tests/hghave.py b/tests/hghave.py
> --- a/tests/hghave.py
> +++ b/tests/hghave.py
> @@ -272,3 +272,29 @@
>       "windows": (has_windows, "Windows"),
>       "msys": (has_msys, "Windows with MSYS"),
>   }
> +
> +class UnknownFeature(Exception):
> +     def __init__(self, f):
> +         self.f = f
> +     def __str__(self):
> +         return self.f

I am not convinced about this use of custom exception types for special 
(but exceptional) return values. It feels like a style we rarely use in 
Mercurial. But it is only the test runner and I don't see a better way 
to do it. I'm in doubt and would like to have a second opinion.

> +def testfeatures(features):
> +
> +    for f in features:
> +        negate = f.startswith('no-')
> +        if negate:
> +            f = f[3:]
> +
> +        if f not in checks:
> +            raise UnknownFeature(f)
> +
> +        check, t = checks[f]
> +        available = check()
> +
> +        if not negate and not available:
> +            return False
> +        elif negate and available:
> +            return False
> +
> +    return True

It seems slightly odd not to share this with 'hghave', but ok, it is 
going to die soon.

> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -54,6 +54,7 @@
>   import time
>   import re
>   import threading
> +import hghave
>   
>   processlock = threading.Lock()
>   
> @@ -597,17 +598,6 @@
>       # True or False when in a true or false conditional section
>       skipping = None
>   
> -    def hghave(reqs):
> -        # TODO: do something smarter when all other uses of hghave is gone
> -        tdir = TESTDIR.replace('\\', '/')
> -        proc = Popen4('%s -c "%s/hghave %s"' %
> -                      (options.shell, tdir, ' '.join(reqs)), wd, 0)
> -        proc.communicate()
> -        ret = proc.wait()
> -        if wifexited(ret):
> -            ret = os.WEXITSTATUS(ret)
> -        return ret == 0
> -
>       f = open(test)
>       t = f.readlines()
>       f.close()
> @@ -623,8 +613,13 @@
>           if l.startswith('#if'):
>               if skipping is not None:
>                   after.setdefault(pos, []).append('  !!! nested #if\n')
> -            skipping = not hghave(l.split()[1:])
>               after.setdefault(pos, []).append(l)
> +            try:
> +                skipping = not hghave.testfeatures(l.split()[1:])

This will run some of the checks in the wrong directory and will thus 
give the wrong result in some cases.

> +            except hghave.UnknownFeature, e:
> +                after.setdefault(pos, []).append(
> +                    '  !!! unknown feature: %s\n' % e)
> +                skipping = True
>           elif l.startswith('#else'):
>               if skipping is None:
>                   after.setdefault(pos, []).append('  !!! missing #if\n')

/Mads