Patchwork [2,of,3] run-tests: support multiple cases in .t test

login
register
mail settings
Submitter Jun Wu
Date May 3, 2017, 2:01 a.m.
Message ID <6663e696ae0c2a1b8f6f.1493776886@x1c>
Download mbox | patch
Permalink /patch/20390/
State Changes Requested
Headers show

Comments

Jun Wu - May 3, 2017, 2:01 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1493488388 25200
#      Sat Apr 29 10:53:08 2017 -0700
# Node ID 6663e696ae0c2a1b8f6fd735dfcf9b99fb4dac73
# Parent  f32a5c7a590fd5d8a9d6c8195b6171331ec9f3a8
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 6663e696ae0c
run-tests: support multiple cases in .t test

Sometimes we want to run similar tests with slightly different
configurations. Previously we duplicate the test files. This patch
introduces special "#case" syntax that allows a single .t file to contain
multiple test cases.

For example, if a test should behave the same with or without an
experimental flag, we can add the following to the .t header:

    #case default
       ....
    #case experimental-a
      $ cat >> $HGRCPATH << EOF
      > [experimental]
      > feature=a
      > EOF
    #endcase

The "experimental-a" block won't be executed when running the "default" test
case.

In the test body, if there are case-dependent outputs, we can also gate them
using the same syntax:

    #case default
      $ hg strip X
      ... save backup bundle ...
    #case experimental-a
      $ hg strip X
      ... pruned ...
    #endcase

The implementation allows "#case" to specify multiple values, to make it
easier to test combinations affected by multiple factors. For example:

    #case A B
      $ flag="$flag --config experimental.factor-a=1"
    #case A C
      $ flag="$flag --config experimental.factor-b=1"
    #case D
    #endcase

A later patch will make use of this feature to simplify test-hardlink*.t.
Pierre-Yves David - May 3, 2017, 1:07 p.m.
I've not looked at the implementation yet, but I'm greatly in favor of 
having this kind of capability available. I found myself needing them on 
a regular basis.

On 05/03/2017 04:01 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1493488388 25200
> #      Sat Apr 29 10:53:08 2017 -0700
> # Node ID 6663e696ae0c2a1b8f6fd735dfcf9b99fb4dac73
> # Parent  f32a5c7a590fd5d8a9d6c8195b6171331ec9f3a8
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 6663e696ae0c
> run-tests: support multiple cases in .t test
>
> Sometimes we want to run similar tests with slightly different
> configurations. Previously we duplicate the test files. This patch
> introduces special "#case" syntax that allows a single .t file to contain
> multiple test cases.
>
> For example, if a test should behave the same with or without an
> experimental flag, we can add the following to the .t header:
>
>     #case default
>        ....
>     #case experimental-a
>       $ cat >> $HGRCPATH << EOF
>       > [experimental]
>       > feature=a
>       > EOF
>     #endcase
>
> The "experimental-a" block won't be executed when running the "default" test
> case.
>
> In the test body, if there are case-dependent outputs, we can also gate them
> using the same syntax:
>
>     #case default
>       $ hg strip X
>       ... save backup bundle ...
>     #case experimental-a
>       $ hg strip X
>       ... pruned ...
>     #endcase
>
> The implementation allows "#case" to specify multiple values, to make it
> easier to test combinations affected by multiple factors. For example:
>
>     #case A B
>       $ flag="$flag --config experimental.factor-a=1"
>     #case A C
>       $ flag="$flag --config experimental.factor-b=1"
>     #case D
>     #endcase
>
> A later patch will make use of this feature to simplify test-hardlink*.t.
>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -592,4 +592,5 @@ class Test(unittest.TestCase):
>          self.name = _strpath(self.bname)
>          self._testdir = os.path.dirname(path)
> +        self._tmpname = os.path.basename(path)
>          self.errpath = os.path.join(self._testdir, b'%s.err' % self.bname)
>
> @@ -651,5 +652,5 @@ class Test(unittest.TestCase):
>                  raise
>
> -        name = os.path.basename(self.path)
> +        name = self._tmpname
>          self._testtmp = os.path.join(self._threadtmp, name)
>          os.mkdir(self._testtmp)
> @@ -1061,4 +1062,16 @@ class TTest(Test):
>      ESCAPEMAP.update({b'\\': b'\\\\', b'\r': br'\r'})
>
> +    def __init__(self, *args, **kwds):
> +        # accept an extra "case" parameter
> +        case = None
> +        if 'case' in kwds:
> +            case = kwds.pop('case')
> +        self._case = case
> +        super(TTest, self).__init__(*args, **kwds)
> +        if case:
> +            self.name += b' (case %s)' % case
> +            self.errpath = b'%s.%s.err' % (self.errpath[:-4], case)
> +            self._tmpname += b'-%s' % case
> +
>      @property
>      def refpath(self):
> @@ -1147,4 +1160,7 @@ class TTest(Test):
>          inpython = False
>
> +        # What "case" we're currently in
> +        incases = None
> +
>          if self._debug:
>              script.append(b'set -x\n')
> @@ -1158,4 +1174,13 @@ class TTest(Test):
>              if not l.endswith(b'\n'):
>                  l += b'\n'
> +            # case handling
> +            if l.startswith(b'#case '):
> +                incases = set(l[6:].split())
> +            elif l.startswith(b'#endcase'):
> +                incases = None
> +            if incases is not None and self._case not in incases:
> +                after.setdefault(pos, []).append(l)
> +                continue
> +            # normal parsing
>              if l.startswith(b'#require'):
>                  lsplit = l.split()
> @@ -2269,7 +2294,27 @@ class TestRunner(object):
>                  args = os.listdir(b'.')
>
> -        return [{'path': t} for t in args
> -                if os.path.basename(t).startswith(b'test-')
> -                    and (t.endswith(b'.py') or t.endswith(b'.t'))]
> +        tests = []
> +        for t in args:
> +            if not (os.path.basename(t).startswith(b'test-')
> +                    and (t.endswith(b'.py') or t.endswith(b'.t'))):
> +                continue
> +            if t.endswith(b'.t'):
> +                # a .t file may contain multiple test cases
> +                # "#case X" indicates a new test case
> +                cases = set()
> +                try:
> +                    with open(t, 'rb') as f:
> +                        for l in f:
> +                            if l.startswith(b'#case '):
> +                                cases.update(l[6:].split())
> +                except IOError:
> +                    pass
> +                if not cases:
> +                    tests.append({'path': t})
> +                else:
> +                    tests += [{'path': t, 'case': c} for c in sorted(cases)]
> +            else:
> +                tests.append({'path': t})
> +        return tests
>
>      def _runtests(self, tests):
> @@ -2277,4 +2322,7 @@ class TestRunner(object):
>              # convert a test back to its description dict
>              desc = {'path': test.path}
> +            case = getattr(test, '_case', None)
> +            if case:
> +                desc['case'] = case
>              return self._gettest(desc, i)
>
> @@ -2375,4 +2423,10 @@ class TestRunner(object):
>          tmpdir = os.path.join(self._hgtmp, b'child%d' % count)
>
> +        # extra keyword parameters. 'case' is used by .t tests
> +        kwds = {}
> +        for key in ['case']:
> +            if key in test:
> +                kwds[key] = test[key]
> +
>          t = testcls(refpath, tmpdir,
>                      keeptmpdir=self.options.keep_tmpdir,
> @@ -2385,5 +2439,5 @@ class TestRunner(object):
>                      hgcommand=self._hgcommand,
>                      usechg=bool(self.options.with_chg or self.options.chg),
> -                    useipv6=useipv6)
> +                    useipv6=useipv6, **kwds)
>          t.should_reload = True
>          return t
> diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
> --- a/tests/test-run-tests.t
> +++ b/tests/test-run-tests.t
> @@ -901,2 +901,51 @@ support for bisecting failed tests autom
>    python hash seed: * (glob)
>    [1]
> +
> +  $ cd ..
> +
> +Test cases in .t files
> +======================
> +  $ mkdir cases
> +  $ cd cases
> +  $ cat > test-cases-abc.t <<'EOF'
> +  > #case B
> +  >   $ V=B
> +  > #case A
> +  >   $ V=A
> +  > #case C
> +  >   $ V=C
> +  > #endcase
> +  >   $ echo $V | sed 's/A/C/'
> +  >   C
> +  > #case C
> +  >   $ [ $V == C ]
> +  > #case A
> +  >   $ [ $V == C ]
> +  >   [1]
> +  > #case A B
> +  >   $ [ $V == C ]
> +  >   [1]
> +  > #endcase
> +  >   $ [ $V == D ]
> +  >   [1]
> +  > EOF
> +  $ rt
> +  .
> +  --- $TESTTMP/anothertests/cases/test-cases-abc.t
> +  +++ $TESTTMP/anothertests/cases/test-cases-abc.t.B.err
> +  @@ -6,7 +6,7 @@
> +     $ V=C
> +   #endcase
> +     $ echo $V | sed 's/A/C/'
> +  -  C
> +  +  B
> +   #case C
> +     $ [ $V == C ]
> +   #case A
> +
> +  ERROR: test-cases-abc.t (case B) output changed
> +  !.
> +  Failed test-cases-abc.t (case B): output changed
> +  # Ran 3 tests, 0 skipped, 0 warned, 1 failed.
> +  python hash seed: * (glob)
> +  [1]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Gregory Szorc - May 4, 2017, 7:07 a.m.
On Tue, May 2, 2017 at 7:01 PM, Jun Wu <quark@fb.com> wrote:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1493488388 25200
> #      Sat Apr 29 10:53:08 2017 -0700
> # Node ID 6663e696ae0c2a1b8f6fd735dfcf9b99fb4dac73
> # Parent  f32a5c7a590fd5d8a9d6c8195b6171331ec9f3a8
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r
> 6663e696ae0c
> run-tests: support multiple cases in .t test
>
> Sometimes we want to run similar tests with slightly different
> configurations. Previously we duplicate the test files. This patch
> introduces special "#case" syntax that allows a single .t file to contain
> multiple test cases.
>
> For example, if a test should behave the same with or without an
> experimental flag, we can add the following to the .t header:
>
>     #case default
>        ....
>     #case experimental-a
>       $ cat >> $HGRCPATH << EOF
>       > [experimental]
>       > feature=a
>       > EOF
>     #endcase
>
> The "experimental-a" block won't be executed when running the "default"
> test
> case.
>
> In the test body, if there are case-dependent outputs, we can also gate
> them
> using the same syntax:
>
>     #case default
>       $ hg strip X
>       ... save backup bundle ...
>     #case experimental-a
>       $ hg strip X
>       ... pruned ...
>     #endcase
>
> The implementation allows "#case" to specify multiple values, to make it
> easier to test combinations affected by multiple factors. For example:
>
>     #case A B
>       $ flag="$flag --config experimental.factor-a=1"
>     #case A C
>       $ flag="$flag --config experimental.factor-b=1"
>     #case D
>     #endcase
>
> A later patch will make use of this feature to simplify test-hardlink*.t.
>

I approve of this feature! This should make it vastly easier to test
configuration variations (especially wire protocol interop).

I performed a half review and don't see anything obviously bad with this
patch: it seems reasonable to me. But I don't have run-tests.py paged in,
so I may have missed something.



>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -592,4 +592,5 @@ class Test(unittest.TestCase):
>          self.name = _strpath(self.bname)
>          self._testdir = os.path.dirname(path)
> +        self._tmpname = os.path.basename(path)
>          self.errpath = os.path.join(self._testdir, b'%s.err' % self.bname)
>
> @@ -651,5 +652,5 @@ class Test(unittest.TestCase):
>                  raise
>
> -        name = os.path.basename(self.path)
> +        name = self._tmpname
>          self._testtmp = os.path.join(self._threadtmp, name)
>          os.mkdir(self._testtmp)
> @@ -1061,4 +1062,16 @@ class TTest(Test):
>      ESCAPEMAP.update({b'\\': b'\\\\', b'\r': br'\r'})
>
> +    def __init__(self, *args, **kwds):
> +        # accept an extra "case" parameter
> +        case = None
> +        if 'case' in kwds:
> +            case = kwds.pop('case')
> +        self._case = case
> +        super(TTest, self).__init__(*args, **kwds)
> +        if case:
> +            self.name += b' (case %s)' % case
> +            self.errpath = b'%s.%s.err' % (self.errpath[:-4], case)
> +            self._tmpname += b'-%s' % case
> +
>      @property
>      def refpath(self):
> @@ -1147,4 +1160,7 @@ class TTest(Test):
>          inpython = False
>
> +        # What "case" we're currently in
> +        incases = None
> +
>          if self._debug:
>              script.append(b'set -x\n')
> @@ -1158,4 +1174,13 @@ class TTest(Test):
>              if not l.endswith(b'\n'):
>                  l += b'\n'
> +            # case handling
> +            if l.startswith(b'#case '):
> +                incases = set(l[6:].split())
> +            elif l.startswith(b'#endcase'):
> +                incases = None
> +            if incases is not None and self._case not in incases:
> +                after.setdefault(pos, []).append(l)
> +                continue
> +            # normal parsing
>              if l.startswith(b'#require'):
>                  lsplit = l.split()
> @@ -2269,7 +2294,27 @@ class TestRunner(object):
>                  args = os.listdir(b'.')
>
> -        return [{'path': t} for t in args
> -                if os.path.basename(t).startswith(b'test-')
> -                    and (t.endswith(b'.py') or t.endswith(b'.t'))]
> +        tests = []
> +        for t in args:
> +            if not (os.path.basename(t).startswith(b'test-')
> +                    and (t.endswith(b'.py') or t.endswith(b'.t'))):
> +                continue
> +            if t.endswith(b'.t'):
> +                # a .t file may contain multiple test cases
> +                # "#case X" indicates a new test case
> +                cases = set()
> +                try:
> +                    with open(t, 'rb') as f:
> +                        for l in f:
> +                            if l.startswith(b'#case '):
> +                                cases.update(l[6:].split())
> +                except IOError:
> +                    pass
> +                if not cases:
> +                    tests.append({'path': t})
> +                else:
> +                    tests += [{'path': t, 'case': c} for c in
> sorted(cases)]
> +            else:
> +                tests.append({'path': t})
> +        return tests
>
>      def _runtests(self, tests):
> @@ -2277,4 +2322,7 @@ class TestRunner(object):
>              # convert a test back to its description dict
>              desc = {'path': test.path}
> +            case = getattr(test, '_case', None)
> +            if case:
> +                desc['case'] = case
>              return self._gettest(desc, i)
>
> @@ -2375,4 +2423,10 @@ class TestRunner(object):
>          tmpdir = os.path.join(self._hgtmp, b'child%d' % count)
>
> +        # extra keyword parameters. 'case' is used by .t tests
> +        kwds = {}
> +        for key in ['case']:
> +            if key in test:
> +                kwds[key] = test[key]
>

Nit: you don't need the for loop here.


> +
>          t = testcls(refpath, tmpdir,
>                      keeptmpdir=self.options.keep_tmpdir,
> @@ -2385,5 +2439,5 @@ class TestRunner(object):
>                      hgcommand=self._hgcommand,
>                      usechg=bool(self.options.with_chg or
> self.options.chg),
> -                    useipv6=useipv6)
> +                    useipv6=useipv6, **kwds)
>          t.should_reload = True
>          return t
> diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
> --- a/tests/test-run-tests.t
> +++ b/tests/test-run-tests.t
> @@ -901,2 +901,51 @@ support for bisecting failed tests autom
>    python hash seed: * (glob)
>    [1]
> +
> +  $ cd ..
> +
> +Test cases in .t files
> +======================
> +  $ mkdir cases
> +  $ cd cases
> +  $ cat > test-cases-abc.t <<'EOF'
> +  > #case B
> +  >   $ V=B
> +  > #case A
> +  >   $ V=A
> +  > #case C
> +  >   $ V=C
> +  > #endcase
> +  >   $ echo $V | sed 's/A/C/'
> +  >   C
> +  > #case C
> +  >   $ [ $V == C ]
> +  > #case A
> +  >   $ [ $V == C ]
> +  >   [1]
> +  > #case A B
> +  >   $ [ $V == C ]
> +  >   [1]
> +  > #endcase
> +  >   $ [ $V == D ]
> +  >   [1]
> +  > EOF
> +  $ rt
> +  .
> +  --- $TESTTMP/anothertests/cases/test-cases-abc.t
> +  +++ $TESTTMP/anothertests/cases/test-cases-abc.t.B.err
> +  @@ -6,7 +6,7 @@
> +     $ V=C
> +   #endcase
> +     $ echo $V | sed 's/A/C/'
> +  -  C
> +  +  B
> +   #case C
> +     $ [ $V == C ]
> +   #case A
> +
> +  ERROR: test-cases-abc.t (case B) output changed
> +  !.
> +  Failed test-cases-abc.t (case B): output changed
> +  # Ran 3 tests, 0 skipped, 0 warned, 1 failed.
> +  python hash seed: * (glob)
> +  [1]
>

Could you please add a test variation running with -v so we have better
test coverage of cases in test names?


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Jun Wu - May 4, 2017, 7:34 p.m.
Excerpts from Gregory Szorc's message of 2017-05-04 00:07:15 -0700:
> I approve of this feature! This should make it vastly easier to test
> configuration variations (especially wire protocol interop).
> 
> I performed a half review and don't see anything obviously bad with this
> patch: it seems reasonable to me. But I don't have run-tests.py paged in,
> so I may have missed something.

It does not handle "-R" correctly. I can fix it in place (which makes this
patch even larger), or send a follow-up with a new test (which may be
cleaner from a small-patch point of view).

> > +        kwds = {}
> > +        for key in ['case']:
> > +            if key in test:
> > +                kwds[key] = test[key]
> >
> 
> Nit: you don't need the for loop here.

This is to avoid duplicating the constant string "key". Maybe it could be
changed to a list comprehension.

> [...]
> 
> Could you please add a test variation running with -v so we have better
> test coverage of cases in test names?

"-v" outputs initialization stuff that is uninteresting to this test and
could be a burden. Since "-v" is not used in test-run-tests.t (maybe for
some reason), I think it's better to not introduce it here.
Yuya Nishihara - May 15, 2017, 1:42 p.m.
On Tue, 2 May 2017 19:01:26 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1493488388 25200
> #      Sat Apr 29 10:53:08 2017 -0700
> # Node ID 6663e696ae0c2a1b8f6fd735dfcf9b99fb4dac73
> # Parent  f32a5c7a590fd5d8a9d6c8195b6171331ec9f3a8
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 6663e696ae0c
> run-tests: support multiple cases in .t test

The idea sounds nice, and the implementation generally looks good to me.
Can you send V2?

> For example, if a test should behave the same with or without an
> experimental flag, we can add the following to the .t header:
> 
>     #case default
>        ....
>     #case experimental-a
>       $ cat >> $HGRCPATH << EOF
>       > [experimental]
>       > feature=a
>       > EOF
>     #endcase

So this is a sort of parameterized tests? I thought "#case" would be just a
syntactic sugar of "#if"s.

Another idea is to define a set of "cases" beforehand, and use the existing
"#if" syntax, but I'm not sure which is better.

 #variants default experimental-a

 #if default
   ...
 #endif

> +                    tests += [{'path': t, 'case': c} for c in sorted(cases)]
> +            else:
> +                tests.append({'path': t})

Instead of extending the test key to a dict, maybe TestRunner._gettest() could
return a list of all cases? If that works, the key handling will get slightly
simpler.

> +  > #case C
> +  >   $ [ $V == C ]
> +  > #case A
> +  >   $ [ $V == C ]
> +  >   [1]
> +  > #case A B
> +  >   $ [ $V == C ]
> +  >   [1]
> +  > #endcase
> +  >   $ [ $V == D ]

Nit '==' is Bashism. Use '='.
Jun Wu - May 15, 2017, 3:18 p.m.
Excerpts from Yuya Nishihara's message of 2017-05-15 22:42:02 +0900:
> [...] 
> So this is a sort of parameterized tests? I thought "#case" would be just a
> syntactic sugar of "#if"s.
> 
> Another idea is to define a set of "cases" beforehand, and use the existing
> "#if" syntax, but I'm not sure which is better.

This feels like implicitly vs explicitly defining a variable. The former
saves several lines (#endif). The latter makes it harder to make mistakes.

It'd be nice if "#if" could support logic operations like "#if a | (b & c)".
Then "#if" could be more powerful.

Since "#if" could be made more powerful in the future, I'll change the code
to use "#if".

>  #variants default experimental-a
>
>  #if default
>    ...
>  #endif

Implementation detail: Maybe "#testcases" is easier to understand.
run-tests.py exports "$HGTESTCASE" so hghave.py could read it.

> > +                    tests += [{'path': t, 'case': c} for c in sorted(cases)]
> > +            else:
> > +                tests.append({'path': t})
> 
> Instead of extending the test key to a dict, maybe TestRunner._gettest() could
> return a list of all cases? If that works, the key handling will get slightly
> simpler.

_gettest is also used by _reloadtest which is intended to reload and return
a single test. We need bi-directional conversion between test instance and
its description dict to make _reloadtest work.

A future feature I was considering is to allow specifying which case to run,
like "run-tests.py a.t:case-a". It might be cleaner to be implemented in
"findtests" since findtests is the only place parsing non-config arguments.

> [...]
Augie Fackler - May 15, 2017, 5:05 p.m.
On Mon, May 15, 2017 at 10:42:02PM +0900, Yuya Nishihara wrote:
> On Tue, 2 May 2017 19:01:26 -0700, Jun Wu wrote:
> > +  > #case C
> > +  >   $ [ $V == C ]
> > +  > #case A
> > +  >   $ [ $V == C ]
> > +  >   [1]
> > +  > #case A B
> > +  >   $ [ $V == C ]
> > +  >   [1]
> > +  > #endcase
> > +  >   $ [ $V == D ]
>
> Nit '==' is Bashism. Use '='.

should I add a check-code rule for something like r'^  ($|>) .*\[[^]]+=='?

(regex untested, wrote it out in my email client)

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Jun Wu - May 15, 2017, 5:46 p.m.
Excerpts from Augie Fackler's message of 2017-05-15 13:05:33 -0400:
> > Nit '==' is Bashism. Use '='.
> 
> should I add a check-code rule for something like r'^  ($|>) .*\[[^]]+=='?
> 
> (regex untested, wrote it out in my email client)

That is a good idea.
Yuya Nishihara - May 16, 2017, 1:17 p.m.
On Mon, 15 May 2017 08:18:22 -0700, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-05-15 22:42:02 +0900:
> > [...] 
> > So this is a sort of parameterized tests? I thought "#case" would be just a
> > syntactic sugar of "#if"s.
> > 
> > Another idea is to define a set of "cases" beforehand, and use the existing
> > "#if" syntax, but I'm not sure which is better.
> 
> This feels like implicitly vs explicitly defining a variable. The former
> saves several lines (#endif). The latter makes it harder to make mistakes.
> 
> It'd be nice if "#if" could support logic operations like "#if a | (b & c)".
> Then "#if" could be more powerful.
> 
> Since "#if" could be made more powerful in the future, I'll change the code
> to use "#if".
> 
> >  #variants default experimental-a
> >
> >  #if default
> >    ...
> >  #endif
> 
> Implementation detail: Maybe "#testcases" is easier to understand.

Yeah, it sounds better.

> run-tests.py exports "$HGTESTCASE" so hghave.py could read it.

TTest has in-process _hghave() function, which would probably be hooked.

> > > +                    tests += [{'path': t, 'case': c} for c in sorted(cases)]
> > > +            else:
> > > +                tests.append({'path': t})
> > 
> > Instead of extending the test key to a dict, maybe TestRunner._gettest() could
> > return a list of all cases? If that works, the key handling will get slightly
> > simpler.
> 
> _gettest is also used by _reloadtest which is intended to reload and return
> a single test. We need bi-directional conversion between test instance and
> its description dict to make _reloadtest work.
> 
> A future feature I was considering is to allow specifying which case to run,
> like "run-tests.py a.t:case-a". It might be cleaner to be implemented in
> "findtests" since findtests is the only place parsing non-config arguments.

Fair enough. Then, using a namedtuple might slightly improve the code
readability. It wasn't easy to read whether the "test" variable is a test
object or a dict.
Jun Wu - May 17, 2017, 4:11 a.m.
Excerpts from Yuya Nishihara's message of 2017-05-16 22:17:18 +0900:
> Fair enough. Then, using a namedtuple might slightly improve the code
> readability. It wasn't easy to read whether the "test" variable is a test
> object or a dict.

namedtuple seems to require all fields to exist, where the "case" is
actually optional. So a dict is more flexible.

I'll update variable names to make the code easier to read.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -592,4 +592,5 @@  class Test(unittest.TestCase):
         self.name = _strpath(self.bname)
         self._testdir = os.path.dirname(path)
+        self._tmpname = os.path.basename(path)
         self.errpath = os.path.join(self._testdir, b'%s.err' % self.bname)
 
@@ -651,5 +652,5 @@  class Test(unittest.TestCase):
                 raise
 
-        name = os.path.basename(self.path)
+        name = self._tmpname
         self._testtmp = os.path.join(self._threadtmp, name)
         os.mkdir(self._testtmp)
@@ -1061,4 +1062,16 @@  class TTest(Test):
     ESCAPEMAP.update({b'\\': b'\\\\', b'\r': br'\r'})
 
+    def __init__(self, *args, **kwds):
+        # accept an extra "case" parameter
+        case = None
+        if 'case' in kwds:
+            case = kwds.pop('case')
+        self._case = case
+        super(TTest, self).__init__(*args, **kwds)
+        if case:
+            self.name += b' (case %s)' % case
+            self.errpath = b'%s.%s.err' % (self.errpath[:-4], case)
+            self._tmpname += b'-%s' % case
+
     @property
     def refpath(self):
@@ -1147,4 +1160,7 @@  class TTest(Test):
         inpython = False
 
+        # What "case" we're currently in
+        incases = None
+
         if self._debug:
             script.append(b'set -x\n')
@@ -1158,4 +1174,13 @@  class TTest(Test):
             if not l.endswith(b'\n'):
                 l += b'\n'
+            # case handling
+            if l.startswith(b'#case '):
+                incases = set(l[6:].split())
+            elif l.startswith(b'#endcase'):
+                incases = None
+            if incases is not None and self._case not in incases:
+                after.setdefault(pos, []).append(l)
+                continue
+            # normal parsing
             if l.startswith(b'#require'):
                 lsplit = l.split()
@@ -2269,7 +2294,27 @@  class TestRunner(object):
                 args = os.listdir(b'.')
 
-        return [{'path': t} for t in args
-                if os.path.basename(t).startswith(b'test-')
-                    and (t.endswith(b'.py') or t.endswith(b'.t'))]
+        tests = []
+        for t in args:
+            if not (os.path.basename(t).startswith(b'test-')
+                    and (t.endswith(b'.py') or t.endswith(b'.t'))):
+                continue
+            if t.endswith(b'.t'):
+                # a .t file may contain multiple test cases
+                # "#case X" indicates a new test case
+                cases = set()
+                try:
+                    with open(t, 'rb') as f:
+                        for l in f:
+                            if l.startswith(b'#case '):
+                                cases.update(l[6:].split())
+                except IOError:
+                    pass
+                if not cases:
+                    tests.append({'path': t})
+                else:
+                    tests += [{'path': t, 'case': c} for c in sorted(cases)]
+            else:
+                tests.append({'path': t})
+        return tests
 
     def _runtests(self, tests):
@@ -2277,4 +2322,7 @@  class TestRunner(object):
             # convert a test back to its description dict
             desc = {'path': test.path}
+            case = getattr(test, '_case', None)
+            if case:
+                desc['case'] = case
             return self._gettest(desc, i)
 
@@ -2375,4 +2423,10 @@  class TestRunner(object):
         tmpdir = os.path.join(self._hgtmp, b'child%d' % count)
 
+        # extra keyword parameters. 'case' is used by .t tests
+        kwds = {}
+        for key in ['case']:
+            if key in test:
+                kwds[key] = test[key]
+
         t = testcls(refpath, tmpdir,
                     keeptmpdir=self.options.keep_tmpdir,
@@ -2385,5 +2439,5 @@  class TestRunner(object):
                     hgcommand=self._hgcommand,
                     usechg=bool(self.options.with_chg or self.options.chg),
-                    useipv6=useipv6)
+                    useipv6=useipv6, **kwds)
         t.should_reload = True
         return t
diff --git a/tests/test-run-tests.t b/tests/test-run-tests.t
--- a/tests/test-run-tests.t
+++ b/tests/test-run-tests.t
@@ -901,2 +901,51 @@  support for bisecting failed tests autom
   python hash seed: * (glob)
   [1]
+
+  $ cd ..
+
+Test cases in .t files
+======================
+  $ mkdir cases
+  $ cd cases
+  $ cat > test-cases-abc.t <<'EOF'
+  > #case B
+  >   $ V=B
+  > #case A
+  >   $ V=A
+  > #case C
+  >   $ V=C
+  > #endcase
+  >   $ echo $V | sed 's/A/C/'
+  >   C
+  > #case C
+  >   $ [ $V == C ]
+  > #case A
+  >   $ [ $V == C ]
+  >   [1]
+  > #case A B
+  >   $ [ $V == C ]
+  >   [1]
+  > #endcase
+  >   $ [ $V == D ]
+  >   [1]
+  > EOF
+  $ rt
+  .
+  --- $TESTTMP/anothertests/cases/test-cases-abc.t
+  +++ $TESTTMP/anothertests/cases/test-cases-abc.t.B.err
+  @@ -6,7 +6,7 @@
+     $ V=C
+   #endcase
+     $ echo $V | sed 's/A/C/'
+  -  C
+  +  B
+   #case C
+     $ [ $V == C ]
+   #case A
+  
+  ERROR: test-cases-abc.t (case B) output changed
+  !.
+  Failed test-cases-abc.t (case B): output changed
+  # Ran 3 tests, 0 skipped, 0 warned, 1 failed.
+  python hash seed: * (glob)
+  [1]