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
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)