Patchwork [RFC] tests: remaining Windows failures

login
register
mail settings
Submitter Katsunori FUJIWARA
Date April 25, 2017, 7:52 p.m.
Message ID <umvb41czd.wl%foozy@lares.dti.ne.jp>
Download mbox | patch
Permalink /patch/20296/
State Not Applicable
Headers show

Comments

Katsunori FUJIWARA - April 25, 2017, 7:52 p.m.
At Wed, 19 Apr 2017 23:04:17 -0400,
Matt Harbison wrote:
> 
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1492656801 14400
> #      Wed Apr 19 22:53:21 2017 -0400
> # Branch stable
> # Node ID 512163f5338fc89676c3f9d6a6b00ae67454ab24
> # Parent  59afb0750aecaff6c2b2e4edaab04eb91eca8a88
> tests: remaining Windows failures
> 

[snip]

> It's unclear to me what test-bookmarks-pushpull is trying to do.  It's not an
> issue with the hook not being directly executable, like some previous fixes.

> diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -364,24 +364,18 @@
>    $ hg -R $TESTTMP/pull-race book
>       @                         1:0d2164f0ce0d
>       X                         1:0d2164f0ce0d
> -   * Y                         5:35d1ef0a8d1b
> +   * Y                         4:b0a5eff05604
>       Z                         1:0d2164f0ce0d

This seems to be executed both for:

  - confirmation of the side effect of "hg pull" in pull-race2
    directory (at line# 332), which advances bookmark "Y" via
    "outgoing" hook

  - confirmation of precondition for "Update a bookmark right after
    the initial lookup -B (issue4689)" from line# 347

But current location (line# 364) makes readers focus mainly on the
latter, and it becomes difficult to understand why bookmark Y is
expected to be advanced, IMHO.

Moving this "hg book" like below may increase understandability.

========================================

This patch also adds "hg -R $TESTTMP/pull-race book" at line# 386, to
confirm the side effect of "hg pull -B ." in pull-race2 directory (at
line# 372), which advances bookmark "Y" via "listkeys.makecommit"
hook.



> If DETACHED_PROCESS is commented out of logtoprocess [3], a new console is shown
> with the output, and the text gets added to the test output properly.  But
> that's probably not the desired behavior.  Should this just #require no-windows?

Current logtoprocess.py implementation makes spawned process share
stdout/stderr with "hg" process, and test-logtoprocess.t says:

    Be sure to append "| cat" to hg commands, to wait for the output,
    if you want to test its output.

Is this sharing desired behavior of "script running asynchronously as
detached daemon processes" ?

If so, "#require no-windows" seems reasonable.


BTW, test-logtoprocess.t has other problems, too.

  - multiple-line configuration problem:

    Multiple-line-ed item in hgrc file contains EOL characters in its
    value.

    On Windows, the 2nd or later lines are completely ignored at
    spawning child process with such value.

    BTW, on POSIX, each lines are executed separately (like normal
    shell script), even if there is no ";" separator at the end of
    each lines.

  - command line separator problem:

    cmd.exe uses only "&" as command line separator

Therefore, current "[logtoprocess]" configuration in
test-logtoprocess.t below doesn't work as expected on Windows :-<

      $ cat >> $HGRCPATH << EOF
      > [extensions]
      > logtoprocess=
      > foocommand=$TESTTMP/foocommand.py
      > [logtoprocess]
      > command=echo 'logtoprocess command output:';
      >     echo "\$EVENT";
      >     echo "\$MSG1";
      >     echo "\$MSG2"
      > commandfinish=echo 'logtoprocess commandfinish output:';
      >     echo "\$EVENT";
      >     echo "\$MSG1";
      >     echo "\$MSG2";
      >     echo "\$MSG3"
      > foo=echo 'logtoprocess foo output:';
      >     echo "\$EVENT";
      >     echo "\$MSG1";
      >     echo "\$OPT_BAR"
      > EOF
Matt Harbison - April 26, 2017, 2:30 a.m.
On Tue, 25 Apr 2017 15:52:38 -0400, FUJIWARA Katsunori  
<foozy@lares.dti.ne.jp> wrote:

> At Wed, 19 Apr 2017 23:04:17 -0400,
> Matt Harbison wrote:
>>
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1492656801 14400
>> #      Wed Apr 19 22:53:21 2017 -0400
>> # Branch stable
>> # Node ID 512163f5338fc89676c3f9d6a6b00ae67454ab24
>> # Parent  59afb0750aecaff6c2b2e4edaab04eb91eca8a88
>> tests: remaining Windows failures
>>
>
> [snip]
>
>> It's unclear to me what test-bookmarks-pushpull is trying to do.  It's  
>> not an
>> issue with the hook not being directly executable, like some previous  
>> fixes.
>
>> diff --git a/tests/test-bookmarks-pushpull.t  
>> b/tests/test-bookmarks-pushpull.t
>> --- a/tests/test-bookmarks-pushpull.t
>> +++ b/tests/test-bookmarks-pushpull.t
>> @@ -364,24 +364,18 @@
>>    $ hg -R $TESTTMP/pull-race book
>>       @                         1:0d2164f0ce0d
>>       X                         1:0d2164f0ce0d
>> -   * Y                         5:35d1ef0a8d1b
>> +   * Y                         4:b0a5eff05604
>>       Z                         1:0d2164f0ce0d
>
> This seems to be executed both for:
>
>   - confirmation of the side effect of "hg pull" in pull-race2
>     directory (at line# 332), which advances bookmark "Y" via
>     "outgoing" hook
>
>   - confirmation of precondition for "Update a bookmark right after
>     the initial lookup -B (issue4689)" from line# 347
>
> But current location (line# 364) makes readers focus mainly on the
> latter, and it becomes difficult to understand why bookmark Y is
> expected to be advanced, IMHO.

Agreed.  It looks like from what I had committed, I missed the first hook  
when I move the hook implementation to a shell script.  It works now that  
both are moved out.

> Moving this "hg book" like below may increase understandability.
>
> ========================================
> diff -r 40cf693fc07d tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -343,6 +343,11 @@
>       X                         1:0d2164f0ce0d
>       Y                         4:b0a5eff05604
>       Z                         1:0d2164f0ce0d
> +  $ hg -R $TESTTMP/pull-race book
> +     @                         1:0d2164f0ce0d
> +     X                         1:0d2164f0ce0d
> +   * Y                         5:35d1ef0a8d1b
> +     Z                         1:0d2164f0ce0d
> Update a bookmark right after the initial lookup -B (issue4689)
> @@ -361,11 +366,6 @@
>    $ hg serve -R ../pull-race -p $HGPORT -d --pid-file=../pull-race.pid  
> -E main-error.log
>    $ cat ../pull-race.pid >> $DAEMON_PIDS
> -  $ hg -R $TESTTMP/pull-race book
> -     @                         1:0d2164f0ce0d
> -     X                         1:0d2164f0ce0d
> -   * Y                         5:35d1ef0a8d1b
> -     Z                         1:0d2164f0ce0d
>    $ hg update -r Y
>    1 files updated, 0 files merged, 1 files removed, 0 files unresolved
>    (activating bookmark Y)
> @@ -383,6 +383,11 @@
>       X                         1:0d2164f0ce0d
>     * Y                         5:35d1ef0a8d1b
>       Z                         1:0d2164f0ce0d
> +  $ hg -R $TESTTMP/pull-race book
> +     @                         1:0d2164f0ce0d
> +     X                         1:0d2164f0ce0d
> +   * Y                         6:0d60821d2197
> +     Z                         1:0d2164f0ce0d
> (done with this section of the test)
> ========================================
>
> This patch also adds "hg -R $TESTTMP/pull-race book" at line# 386, to
> confirm the side effect of "hg pull -B ." in pull-race2 directory (at
> line# 372), which advances bookmark "Y" via "listkeys.makecommit"
> hook.
>
>
>
>> If DETACHED_PROCESS is commented out of logtoprocess [3], a new console  
>> is shown
>> with the output, and the text gets added to the test output properly.   
>> But
>> that's probably not the desired behavior.  Should this just #require  
>> no-windows?
>
> Current logtoprocess.py implementation makes spawned process share
> stdout/stderr with "hg" process, and test-logtoprocess.t says:
>
>     Be sure to append "| cat" to hg commands, to wait for the output,
>     if you want to test its output.
>
> Is this sharing desired behavior of "script running asynchronously as
> detached daemon processes" ?
>
> If so, "#require no-windows" seems reasonable.

I'm fine with that.  I think this is a FB extension, so I'll wait until  
one of their devs chime in.

>
> BTW, test-logtoprocess.t has other problems, too.
>
>   - multiple-line configuration problem:
>
>     Multiple-line-ed item in hgrc file contains EOL characters in its
>     value.
>
>     On Windows, the 2nd or later lines are completely ignored at
>     spawning child process with such value.
>
>     BTW, on POSIX, each lines are executed separately (like normal
>     shell script), even if there is no ";" separator at the end of
>     each lines.
>
>   - command line separator problem:
>
>     cmd.exe uses only "&" as command line separator
>
> Therefore, current "[logtoprocess]" configuration in
> test-logtoprocess.t below doesn't work as expected on Windows :-<
>
>       $ cat >> $HGRCPATH << EOF
>       > [extensions]
>       > logtoprocess=
>       > foocommand=$TESTTMP/foocommand.py
>       > [logtoprocess]
>       > command=echo 'logtoprocess command output:';
>       >     echo "\$EVENT";
>       >     echo "\$MSG1";
>       >     echo "\$MSG2"
>       > commandfinish=echo 'logtoprocess commandfinish output:';
>       >     echo "\$EVENT";
>       >     echo "\$MSG1";
>       >     echo "\$MSG2";
>       >     echo "\$MSG3"
>       > foo=echo 'logtoprocess foo output:';
>       >     echo "\$EVENT";
>       >     echo "\$MSG1";
>       >     echo "\$OPT_BAR"
>       > EOF
>
>

Patch

========================================
diff -r 40cf693fc07d tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -343,6 +343,11 @@ 
      X                         1:0d2164f0ce0d
      Y                         4:b0a5eff05604
      Z                         1:0d2164f0ce0d
+  $ hg -R $TESTTMP/pull-race book
+     @                         1:0d2164f0ce0d
+     X                         1:0d2164f0ce0d
+   * Y                         5:35d1ef0a8d1b
+     Z                         1:0d2164f0ce0d
 
 Update a bookmark right after the initial lookup -B (issue4689)
 
@@ -361,11 +366,6 @@ 
   $ hg serve -R ../pull-race -p $HGPORT -d --pid-file=../pull-race.pid -E main-error.log
   $ cat ../pull-race.pid >> $DAEMON_PIDS
 
-  $ hg -R $TESTTMP/pull-race book
-     @                         1:0d2164f0ce0d
-     X                         1:0d2164f0ce0d
-   * Y                         5:35d1ef0a8d1b
-     Z                         1:0d2164f0ce0d
   $ hg update -r Y
   1 files updated, 0 files merged, 1 files removed, 0 files unresolved
   (activating bookmark Y)
@@ -383,6 +383,11 @@ 
      X                         1:0d2164f0ce0d
    * Y                         5:35d1ef0a8d1b
      Z                         1:0d2164f0ce0d
+  $ hg -R $TESTTMP/pull-race book
+     @                         1:0d2164f0ce0d
+     X                         1:0d2164f0ce0d
+   * Y                         6:0d60821d2197
+     Z                         1:0d2164f0ce0d
 
 (done with this section of the test)