Patchwork [2,of,2] tests: fix run-tests when there's a bad #if in a test

login
register
mail settings
Submitter Matt Harbison
Date June 5, 2017, 3:56 a.m.
Message ID <op.y1cc7mz59lwrgf@envy>
Download mbox | patch
Permalink /patch/21206/
State Not Applicable
Headers show

Comments

Matt Harbison - June 5, 2017, 3:56 a.m.
On Tue, 30 May 2017 21:12:31 -0400, Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1496191723 14400
> #      Tue May 30 20:48:43 2017 -0400
> # Node ID 48ea485539377729a711959c15ac4262550234fc
> # Parent  361bf17d190dbd3410be4b7fd8ff4d27ddb48739
> tests: fix run-tests when there's a bad #if in a test
>
> That has (and still does) caused the test to be skipped, but without
> this fix it was possible to exit this block of code without clearing
> the output channel, which poisoned the channel list for later test
> method runs. Fix this by always clearing the channel in a finally.
>
> The test for this is somewhat unfortunate. Sadly, I couldn't get a way
> to reproduce this with less than 2n+1 test cases, nor could I get it
> to reproduce reliably without the sleep statements. It's also crucial
> that the test with the broken #if be smaller (in terms of byte count)
> than the sleeping tests, so that it runs first and would poison the
> channel list prior to another test needing that entry from the list.

Any thoughts about this Windows failure?  Unlike the recent SSH fix where  
the output order only changed sometimes, this seems consistent.


> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -1778,10 +1778,11 @@ class TestSuite(unittest.TestSuite):
>              except: # re-raises
>                  done.put(('!', test, 'run-test raised an error, see  
> traceback'))
>                  raise
> -            try:
> -                channels[channel] = ''
> -            except IndexError:
> -                pass
> +            finally:
> +                try:
> +                    channels[channel] = ''
> +                except IndexError:
> +                    pass
>         def stat():
>              count = 0
> 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
> @@ -903,6 +903,30 @@ support for bisecting failed tests autom
>   $ cd ..
> +Test a broken #if statement doesn't break run-tests threading.
> +==============================================================
> +  $ mkdir broken
> +  $ cd broken
> +  $ cat > test-broken.t <<EOF
> +  > true
> +  > #if notarealhghavefeature
> +  >   $ false
> +  > #endif
> +  > EOF
> +  $ for f in 1 2 3 4 ; do
> +  > cat > test-works-$f.t <<EOF
> +  > This is test case $f
> +  >   $ sleep 1
> +  > EOF
> +  > done
> +  $ rt -j 2
> +  ....
> +  # Ran 5 tests, 0 skipped, 0 warned, 0 failed.
> +  skipped: unknown feature: notarealhghavefeature
> +
> +  $ cd ..
> +  $ rm -rf broken
> +
>  Test cases in .t files
>  ======================
>    $ mkdir cases
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - June 5, 2017, 1:14 p.m.
> On Jun 4, 2017, at 23:56, Matt Harbison <mharbison72@gmail.com> wrote:
> 
> On Tue, 30 May 2017 21:12:31 -0400, Augie Fackler <raf@durin42.com> wrote:
> 
>> # HG changeset patch
>> # User Augie Fackler <augie@google.com>
>> # Date 1496191723 14400
>> #      Tue May 30 20:48:43 2017 -0400
>> # Node ID 48ea485539377729a711959c15ac4262550234fc
>> # Parent  361bf17d190dbd3410be4b7fd8ff4d27ddb48739
>> tests: fix run-tests when there's a bad #if in a test
>> 
>> That has (and still does) caused the test to be skipped, but without
>> this fix it was possible to exit this block of code without clearing
>> the output channel, which poisoned the channel list for later test
>> method runs. Fix this by always clearing the channel in a finally.
>> 
>> The test for this is somewhat unfortunate. Sadly, I couldn't get a way
>> to reproduce this with less than 2n+1 test cases, nor could I get it
>> to reproduce reliably without the sleep statements. It's also crucial
>> that the test with the broken #if be smaller (in terms of byte count)
>> than the sleeping tests, so that it runs first and would poison the
>> channel list prior to another test needing that entry from the list.
> 
> Any thoughts about this Windows failure?  Unlike the recent SSH fix where the output order only changed sometimes, this seems consistent.
> 
> 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
> @@ -920,10 +920,10 @@
>   > EOF
>   > done
>   $ rt -j 2
> -  ....
> +  ....skipped: unknown feature: notarealhghavefeature\r (esc)
> +
> +
>   # Ran 5 tests, 0 skipped, 0 warned, 0 failed.
> -  skipped: unknown feature: notarealhghavefeature
> -
>   $ cd ..
>   $ rm -rf broken

Looks like it's probably some sort of locking issue in run-tests.py. Ugh.

> 
> 
>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>> --- a/tests/run-tests.py
>> +++ b/tests/run-tests.py
>> @@ -1778,10 +1778,11 @@ class TestSuite(unittest.TestSuite):
>>             except: # re-raises
>>                 done.put(('!', test, 'run-test raised an error, see traceback'))
>>                 raise
>> -            try:
>> -                channels[channel] = ''
>> -            except IndexError:
>> -                pass
>> +            finally:
>> +                try:
>> +                    channels[channel] = ''
>> +                except IndexError:
>> +                    pass
>>        def stat():
>>             count = 0
>> 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
>> @@ -903,6 +903,30 @@ support for bisecting failed tests autom
>>  $ cd ..
>> +Test a broken #if statement doesn't break run-tests threading.
>> +==============================================================
>> +  $ mkdir broken
>> +  $ cd broken
>> +  $ cat > test-broken.t <<EOF
>> +  > true
>> +  > #if notarealhghavefeature
>> +  >   $ false
>> +  > #endif
>> +  > EOF
>> +  $ for f in 1 2 3 4 ; do
>> +  > cat > test-works-$f.t <<EOF
>> +  > This is test case $f
>> +  >   $ sleep 1
>> +  > EOF
>> +  > done
>> +  $ rt -j 2
>> +  ....
>> +  # Ran 5 tests, 0 skipped, 0 warned, 0 failed.
>> +  skipped: unknown feature: notarealhghavefeature
>> +
>> +  $ cd ..
>> +  $ rm -rf broken
>> +
>> Test cases in .t files
>> ======================
>>   $ mkdir cases
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

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
@@ -920,10 +920,10 @@ 
    > EOF
    > done
    $ rt -j 2
-  ....
+  ....skipped: unknown feature: notarealhghavefeature\r (esc)
+
+
    # Ran 5 tests, 0 skipped, 0 warned, 0 failed.
-  skipped: unknown feature: notarealhghavefeature
-
    $ cd ..
    $ rm -rf broken