On Tue, 25 Apr 2017 15:52:38 -0400, FUJIWARA Katsunori <fo...@lares.dti.ne.jp> wrote:

At Wed, 19 Apr 2017 23:04:17 -0400,
Matt Harbison wrote:

# HG changeset patch
# User Matt Harbison <matt_harbi...@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


_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to