Re: [PATCH RFC] tests: remaining Windows failures

[Matt Harbison]

On Tue, 25 Apr 2017 15:52:38 -0400, FUJIWARA Katsunori  
 wrote:



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


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

Re: [PATCH RFC] tests: remaining Windows failures

[FUJIWARA Katsunori]

At Wed, 19 Apr 2017 23:04:17 -0400,
Matt Harbison wrote:
> 
> # HG changeset patch
> # User Matt Harbison 
> # 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.


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.


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";
  >  

Re: [PATCH RFC] tests: remaining Windows failures

[Matt Harbison]

On Thu, 20 Apr 2017 10:42:50 -0400, Jun Wu  wrote:


Excerpts from Matt Harbison's message of 2017-04-19 23:04:17 -0400:

[...]
diff --git a/tests/test-worker.t b/tests/test-worker.t
--- a/tests/test-worker.t
+++ b/tests/test-worker.t
@@ -81,7 +81,6 @@
   $ hg --config "extensions.t=$abspath" --config 'worker.numcpus=2' \
   > test 10.0 abort --traceback 2>&1 | grep '^Traceback'
   Traceback (most recent call last):
-  Traceback (most recent call last):


This is because worker only works for POSIX. Related code are:

if pycompat.osname == 'posix':
_startupcost = 0.01
else:
_startupcost = 1e30

def worthwhile(ui, costperop, nops):
'''try to determine whether the benefit of multiple processes can
outweigh the cost of starting them'''
linear = costperop * nops
workers = _numworkers(ui)
benefit = linear - (_startupcost * workers + linear / workers)
return benefit >= 0.15

worthwhile return False because benefit is negative.

if pycompat.osname != 'nt':
_platformworker = _posixworker
_exitstatus = _posixexitstatus

_platformworker is undefined on Windows.


Thanks for the detailed explanation.





 Traceback must be printed for unknown exceptions

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

Re: [PATCH RFC] tests: remaining Windows failures

[Jun Wu]

Excerpts from Matt Harbison's message of 2017-04-19 23:04:17 -0400:
> [...]
> diff --git a/tests/test-worker.t b/tests/test-worker.t
> --- a/tests/test-worker.t
> +++ b/tests/test-worker.t
> @@ -81,7 +81,6 @@
>$ hg --config "extensions.t=$abspath" --config 'worker.numcpus=2' \
>> test 10.0 abort --traceback 2>&1 | grep '^Traceback'
>Traceback (most recent call last):
> -  Traceback (most recent call last):

This is because worker only works for POSIX. Related code are:

if pycompat.osname == 'posix':
_startupcost = 0.01
else:
_startupcost = 1e30

def worthwhile(ui, costperop, nops):
'''try to determine whether the benefit of multiple processes can
outweigh the cost of starting them'''
linear = costperop * nops
workers = _numworkers(ui)
benefit = linear - (_startupcost * workers + linear / workers)
return benefit >= 0.15

worthwhile return False because benefit is negative.

if pycompat.osname != 'nt':
_platformworker = _posixworker
_exitstatus = _posixexitstatus

_platformworker is undefined on Windows.

>  
>  Traceback must be printed for unknown exceptions
  
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel