Patchwork [2,of,2,v2] tests: rename syshg to testrepohg

login
register
mail settings
Submitter Adam Simpkins
Date July 1, 2017, 1:21 a.m.
Message ID <8fbab57de1136dbbb839.1498872071@devbig125.prn1.facebook.com>
Download mbox | patch
Permalink /patch/21857/
State Superseded
Headers show

Comments

Adam Simpkins - July 1, 2017, 1:21 a.m.
# HG changeset patch
# User Adam Simpkins <simpkins@fb.com>
# Date 1498871800 25200
#      Fri Jun 30 18:16:40 2017 -0700
# Node ID 8fbab57de1136dbbb8397959785945255352d200
# Parent  bc44cbbfd13d88fbd2d10a7eed7c0ea26002c87c
tests: rename syshg to testrepohg

Rename the syshg function to testrepohg, to more accurately reflect what this
function does now.  This function runs hg for interacting with the $TESTDIR/..
repository.  This intelligently decides whether to use the local hg script or
the system hg installation.
Matt Harbison - July 1, 2017, 2:04 a.m.
On Fri, 30 Jun 2017 21:21:11 -0400, Adam Simpkins <simpkins@fb.com> wrote:

> # HG changeset patch
> # User Adam Simpkins <simpkins@fb.com>
> # Date 1498871800 25200
> #      Fri Jun 30 18:16:40 2017 -0700
> # Node ID 8fbab57de1136dbbb8397959785945255352d200
> # Parent  bc44cbbfd13d88fbd2d10a7eed7c0ea26002c87c
> tests: rename syshg to testrepohg
>
> Rename the syshg function to testrepohg, to more accurately reflect what  
> this
> function does now.  This function runs hg for interacting with the  
> $TESTDIR/..
> repository.  This intelligently decides whether to use the local hg  
> script or
> the system hg installation.

This fixes the problem in V1, but reintroduces the obsolete markers line.   
Oddly, only in test-check-commit.t, out of all of the *check* tests.

Any diagnostics you'd like me to collect?
Adam Simpkins - July 1, 2017, 2:22 a.m.
On Jun 30, Matt Harbison wrote:
> On Fri, 30 Jun 2017 21:21:11 -0400, Adam Simpkins <simpkins@fb.com> wrote:
> 
> ># HG changeset patch
> ># User Adam Simpkins <simpkins@fb.com>
> ># Date 1498871800 25200
> >#      Fri Jun 30 18:16:40 2017 -0700
> ># Node ID 8fbab57de1136dbbb8397959785945255352d200
> ># Parent  bc44cbbfd13d88fbd2d10a7eed7c0ea26002c87c
> >tests: rename syshg to testrepohg
> >
> >Rename the syshg function to testrepohg, to more accurately
> >reflect what this
> >function does now.  This function runs hg for interacting with the
> >$TESTDIR/..
> >repository.  This intelligently decides whether to use the local
> >hg script or
> >the system hg installation.
> 
> This fixes the problem in V1, but reintroduces the obsolete markers
> line.  Oddly, only in test-check-commit.t, out of all of the *check*
> tests.
> 
> Any diagnostics you'd like me to collect?

Hmm, I suspect that's because the first patch in this set restored the
default behavior of trying the local hg script first, and only using
the system hg if that fails, as Yuya requested.

Using the local hg script "works" in this case (has a 0 exit status),
but prints a warnings since the settings aren't quite right.

Personally I think the right behavior here is to default to the system
hg first, and only use the local hg script if that fails.  The system
hg command is almost certainly the right thing to use--if we try using
the local hg script with the test's HGRCPATH settings we are much more
likely to be using the wrong settings for this repository.

I'll send out an updated patch series that goes back to using the
system hg first.  We could also update the fallback case to always
pass in "--config evolution=createmarkers" when using the local hg
script to try and avoid this particular warning, but this feels like a
hack.
Matt Harbison - July 1, 2017, 2:36 a.m.
On Fri, 30 Jun 2017 22:22:42 -0400, Adam Simpkins <simpkins@fb.com> wrote:

> On Jun 30, Matt Harbison wrote:
>> On Fri, 30 Jun 2017 21:21:11 -0400, Adam Simpkins <simpkins@fb.com>  
>> wrote:
>>
>> ># HG changeset patch
>> ># User Adam Simpkins <simpkins@fb.com>
>> ># Date 1498871800 25200
>> >#      Fri Jun 30 18:16:40 2017 -0700
>> ># Node ID 8fbab57de1136dbbb8397959785945255352d200
>> ># Parent  bc44cbbfd13d88fbd2d10a7eed7c0ea26002c87c
>> >tests: rename syshg to testrepohg
>> >
>> >Rename the syshg function to testrepohg, to more accurately
>> >reflect what this
>> >function does now.  This function runs hg for interacting with the
>> >$TESTDIR/..
>> >repository.  This intelligently decides whether to use the local
>> >hg script or
>> >the system hg installation.
>>
>> This fixes the problem in V1, but reintroduces the obsolete markers
>> line.  Oddly, only in test-check-commit.t, out of all of the *check*
>> tests.
>>
>> Any diagnostics you'd like me to collect?
>
> Hmm, I suspect that's because the first patch in this set restored the
> default behavior of trying the local hg script first, and only using
> the system hg if that fails, as Yuya requested.
>
> Using the local hg script "works" in this case (has a 0 exit status),
> but prints a warnings since the settings aren't quite right.
>
> Personally I think the right behavior here is to default to the system
> hg first, and only use the local hg script if that fails.  The system
> hg command is almost certainly the right thing to use--if we try using
> the local hg script with the test's HGRCPATH settings we are much more
> likely to be using the wrong settings for this repository.

Since I've got all of my extensions enabled in my user level config file,  
I can't get my mind around why local vs system hg would matter (for me).

Is it possible to unset HGRCPATH, so that the system default configs are  
used?  FWIW, I generally keep my system hg within a few releases of the  
latest.  That way I can do updates without the code modifying itself on  
the fly.  Otherwise, I'm using the local hg.

> I'll send out an updated patch series that goes back to using the
> system hg first.  We could also update the fallback case to always
> pass in "--config evolution=createmarkers" when using the local hg
> script to try and avoid this particular warning, but this feels like a
> hack.

That's equivalent to what it was before, so I'm fine with that too.
Adam Simpkins - July 1, 2017, 2:44 a.m.
On Jun 30, Matt Harbison wrote:
> On Fri, 30 Jun 2017 22:22:42 -0400, Adam Simpkins <simpkins@fb.com> wrote:
> 
> >On Jun 30, Matt Harbison wrote:
> >>On Fri, 30 Jun 2017 21:21:11 -0400, Adam Simpkins
> >><simpkins@fb.com> wrote:
> >>
> >>># HG changeset patch
> >>># User Adam Simpkins <simpkins@fb.com>
> >>># Date 1498871800 25200
> >>>#      Fri Jun 30 18:16:40 2017 -0700
> >>># Node ID 8fbab57de1136dbbb8397959785945255352d200
> >>># Parent  bc44cbbfd13d88fbd2d10a7eed7c0ea26002c87c
> >>>tests: rename syshg to testrepohg
> >>>
> >>>Rename the syshg function to testrepohg, to more accurately
> >>>reflect what this
> >>>function does now.  This function runs hg for interacting with the
> >>>$TESTDIR/..
> >>>repository.  This intelligently decides whether to use the local
> >>>hg script or
> >>>the system hg installation.
> >>
> >>This fixes the problem in V1, but reintroduces the obsolete markers
> >>line.  Oddly, only in test-check-commit.t, out of all of the *check*
> >>tests.
> >>
> >>Any diagnostics you'd like me to collect?
> >
> >Hmm, I suspect that's because the first patch in this set restored the
> >default behavior of trying the local hg script first, and only using
> >the system hg if that fails, as Yuya requested.
> >
> >Using the local hg script "works" in this case (has a 0 exit status),
> >but prints a warnings since the settings aren't quite right.
> >
> >Personally I think the right behavior here is to default to the system
> >hg first, and only use the local hg script if that fails.  The system
> >hg command is almost certainly the right thing to use--if we try using
> >the local hg script with the test's HGRCPATH settings we are much more
> >likely to be using the wrong settings for this repository.
> 
> Since I've got all of my extensions enabled in my user level config
> file, I can't get my mind around why local vs system hg would matter
> (for me).
> 
> Is it possible to unset HGRCPATH, so that the system default configs
> are used?  FWIW, I generally keep my system hg within a few releases
> of the latest.  That way I can do updates without the code modifying
> itself on the fly.  Otherwise, I'm using the local hg.

That's effectively what the syshg version is trying to do:
it tries to invoke "hg" as closely as possible to how you would invoke
it manually.  It uses the same HGRCPATH settings you do, and the same
version of hg that you get from your PATH.

I think just doing this behavior by default is the right approach, and
only falling back to the local hg script if the hg from $PATH is so
outdated that we cannot use it.  This behavior should do the right
thing for pretty much everyone I believe.

The v1 and v2 versions of my patch reverted the behavior to use the
local hg script as long as it worked, but they were a bit too lenient
with how they detected if it worked or not.  In your case they decided
it was good enough to use even though it printed warnings in some cases.
The v3 version of the patch goes back to using the system hg by
default, and only falling back to the local hg as a last resort if
that is completely unusable.

Patch

diff --git a/tests/helpers-testrepo.sh b/tests/helpers-testrepo.sh
--- a/tests/helpers-testrepo.sh
+++ b/tests/helpers-testrepo.sh
@@ -1,4 +1,4 @@ 
-# This file defines syshg and syshgenv functions to use for calling
+# This file defines testrepohg and testrepohgenv functions to use for calling
 # hg on the hg repository itself ($TESTDIR/..)
 #
 # Interacting with the hg repository may need to be done differently than
@@ -18,15 +18,15 @@ 
 #
 # If neither of these work, we exit with status 80 to skip the test.
 
-realsyshgenv () {
+syshgenv () {
     . "$HGTEST_RESTOREENV"
     HGPLAIN=1
     export HGPLAIN
 }
 
-realsyshg () {
+syshg () {
     (
-        realsyshgenv
+        syshgenv
         exec hg "$@"
     )
 }
@@ -34,21 +34,21 @@ 
 if hg --cwd "$TESTDIR/.." log -r. -Ttest >/dev/null 2>/dev/null; then
     # If our local hg command works with the test HGRCPATH settings,
     # just use it.
-    syshgenv () {
+    testrepohgenv () {
         :
     }
-    syshg () {
+    testrepohg () {
         hg "$@"
     }
-elif realsyshg --cwd "$TESTDIR/.." log -r. -Ttest >/dev/null 2>/dev/null && \
-     realsyshg files -h >/dev/null 2>/dev/null; then
+elif syshg --cwd "$TESTDIR/.." log -r. -Ttest >/dev/null 2>/dev/null && \
+     syshg files -h >/dev/null 2>/dev/null; then
     # Otherwise, if the local hg command does not work but hg from $PATH works
     # with an unmodified $HGRCPATH, use it.
-    syshgenv () {
-        realsyshgenv
+    testrepohgenv () {
+        syshgenv
     }
-    syshg () {
-        realsyshg "$@"
+    testrepohg () {
+        syshg "$@"
     }
 else
     # We couldn't find an hg installation capable of interacting with
diff --git a/tests/test-check-code.t b/tests/test-check-code.t
--- a/tests/test-check-code.t
+++ b/tests/test-check-code.t
@@ -7,7 +7,7 @@ 
 New errors are not allowed. Warnings are strongly discouraged.
 (The writing "no-che?k-code" is for not skipping this file when checking.)
 
-  $ syshg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman |
+  $ testrepohg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman |
   > sed 's-\\-/-g' | "$check_code" --warnings --per-file=0 - || false
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
@@ -33,7 +33,7 @@ 
 
 Prevent adding new files in the root directory accidentally.
 
-  $ syshg files 'glob:*'
+  $ testrepohg files 'glob:*'
   .editorconfig
   .hgignore
   .hgsigs
diff --git a/tests/test-check-commit.t b/tests/test-check-commit.t
--- a/tests/test-check-commit.t
+++ b/tests/test-check-commit.t
@@ -8,8 +8,8 @@ 
 
   $ cd $TESTDIR/..
 
-  $ for node in `syshg log --rev 'not public() and ::. and not desc("# no-check-commit")' --template '{node|short}\n'`; do
-  >    syshg export --git $node | contrib/check-commit > ${TESTTMP}/check-commit.out
+  $ for node in `testrepohg log --rev 'not public() and ::. and not desc("# no-check-commit")' --template '{node|short}\n'`; do
+  >    testrepohg export --git $node | contrib/check-commit > ${TESTTMP}/check-commit.out
   >    if [ $? -ne 0 ]; then
   >        echo "Revision $node does not comply with rules"
   >        echo '------------------------------------------------------'
diff --git a/tests/test-check-config.t b/tests/test-check-config.t
--- a/tests/test-check-config.t
+++ b/tests/test-check-config.t
@@ -31,7 +31,7 @@ 
 
 New errors are not allowed. Warnings are strongly discouraged.
 
-  $ syshg files "set:(**.py or **.txt) - tests/**" | sed 's|\\|/|g' |
+  $ testrepohg files "set:(**.py or **.txt) - tests/**" | sed 's|\\|/|g' |
   >   $PYTHON contrib/check-config.py
               limit = ui.configwith(fraction, 'profiling', 'showmin', 0.05)
   
diff --git a/tests/test-check-execute.t b/tests/test-check-execute.t
--- a/tests/test-check-execute.t
+++ b/tests/test-check-execute.t
@@ -5,20 +5,20 @@ 
 
 look for python scripts without the execute bit
 
-  $ syshg files 'set:**.py and not exec() and grep(r"^#!.*?python")'
+  $ testrepohg files 'set:**.py and not exec() and grep(r"^#!.*?python")'
   [1]
 
 look for python scripts with execute bit but not shebang
 
-  $ syshg files 'set:**.py and exec() and not grep(r"^#!.*?python")'
+  $ testrepohg files 'set:**.py and exec() and not grep(r"^#!.*?python")'
   [1]
 
 look for shell scripts with execute bit but not shebang
 
-  $ syshg files 'set:**.sh and exec() and not grep(r"^#!.*(ba)?sh")'
+  $ testrepohg files 'set:**.sh and exec() and not grep(r"^#!.*(ba)?sh")'
   [1]
 
 look for non scripts with no shebang
 
-  $ syshg files 'set:exec() and not **.sh and not **.py and not grep(r"^#!")'
+  $ testrepohg files 'set:exec() and not **.sh and not **.py and not grep(r"^#!")'
   [1]
diff --git a/tests/test-check-help.t b/tests/test-check-help.t
--- a/tests/test-check-help.t
+++ b/tests/test-check-help.t
@@ -23,7 +23,7 @@ 
 Check if ":hg:`help TOPIC`" is valid:
 (use "xargs -n1 -t" to see which help commands are executed)
 
-  $ syshg files 'glob:{hgdemandimport,hgext,mercurial}/**/*.py' \
+  $ testrepohg files 'glob:{hgdemandimport,hgext,mercurial}/**/*.py' \
   > | sed 's|\\|/|g' \
   > | xargs $PYTHON "$TESTTMP/scanhelptopics.py" \
   > | xargs -n1 hg help > /dev/null
diff --git a/tests/test-check-module-imports.t b/tests/test-check-module-imports.t
--- a/tests/test-check-module-imports.t
+++ b/tests/test-check-module-imports.t
@@ -14,7 +14,7 @@ 
 Known-bad files are excluded by -X as some of them would produce unstable
 outputs, which should be fixed later.
 
-  $ syshg locate 'set:**.py or grep(r"^#!.*?python")' \
+  $ testrepohg locate 'set:**.py or grep(r"^#!.*?python")' \
   > 'tests/**.t' \
   > -X contrib/debugshell.py \
   > -X contrib/python-zstandard/ \
diff --git a/tests/test-check-py3-compat.t b/tests/test-check-py3-compat.t
--- a/tests/test-check-py3-compat.t
+++ b/tests/test-check-py3-compat.t
@@ -3,7 +3,7 @@ 
   $ . "$TESTDIR/helpers-testrepo.sh"
   $ cd "$TESTDIR"/..
 
-  $ syshg files 'set:(**.py)' | sed 's|\\|/|g' | xargs $PYTHON contrib/check-py3-compat.py
+  $ testrepohg files 'set:(**.py)' | sed 's|\\|/|g' | xargs $PYTHON contrib/check-py3-compat.py
   contrib/python-zstandard/setup.py not using absolute_import
   contrib/python-zstandard/setup_zstd.py not using absolute_import
   contrib/python-zstandard/tests/common.py not using absolute_import
@@ -22,7 +22,7 @@ 
   tests/test-demandimport.py not using absolute_import
 
 #if py3exe
-  $ syshg files 'set:(**.py) - grep(pygments)' -X hgext/fsmonitor/pywatchman \
+  $ testrepohg files 'set:(**.py) - grep(pygments)' -X hgext/fsmonitor/pywatchman \
   > | sed 's|\\|/|g' | xargs $PYTHON3 contrib/check-py3-compat.py \
   > | sed 's/[0-9][0-9]*)$/*)/'
   hgext/convert/transport.py: error importing: <*Error> No module named 'svn.client' (error at transport.py:*) (glob)
@@ -38,7 +38,7 @@ 
 #endif
 
 #if py3exe py3pygments
-  $ syshg files 'set:(**.py) and grep(pygments)' | sed 's|\\|/|g' \
+  $ testrepohg files 'set:(**.py) and grep(pygments)' | sed 's|\\|/|g' \
   > | xargs $PYTHON3 contrib/check-py3-compat.py \
   > | sed 's/[0-9][0-9]*)$/*)/'
 #endif
diff --git a/tests/test-check-pyflakes.t b/tests/test-check-pyflakes.t
--- a/tests/test-check-pyflakes.t
+++ b/tests/test-check-pyflakes.t
@@ -6,7 +6,7 @@ 
 run pyflakes on all tracked files ending in .py or without a file ending
 (skipping binary file random-seed)
 
-  $ syshg locate 'set:**.py or grep("^#!.*python")' \
+  $ testrepohg locate 'set:**.py or grep("^#!.*python")' \
   > -X hgext/fsmonitor/pywatchman \
   > -X mercurial/pycompat.py -X contrib/python-zstandard \
   > 2>/dev/null \
diff --git a/tests/test-check-shbang.t b/tests/test-check-shbang.t
--- a/tests/test-check-shbang.t
+++ b/tests/test-check-shbang.t
@@ -5,11 +5,11 @@ 
 
 look for python scripts that do not use /usr/bin/env
 
-  $ syshg files 'set:grep(r"^#!.*?python") and not grep(r"^#!/usr/bi{1}n/env python") - **/*.t'
+  $ testrepohg files 'set:grep(r"^#!.*?python") and not grep(r"^#!/usr/bi{1}n/env python") - **/*.t'
   [1]
 
 In tests, enforce $PYTHON and *not* /usr/bin/env python or similar:
-  $ syshg files 'set:grep(r"#!.*?python") and **/*.t' \
+  $ testrepohg files 'set:grep(r"#!.*?python") and **/*.t' \
   > -X tests/test-check-execute.t \
   > -X tests/test-check-module-imports.t \
   > -X tests/test-check-pyflakes.t \
@@ -21,5 +21,5 @@ 
 
 look for shell scripts that do not use /bin/sh
 
-  $ syshg files 'set:grep(r"^#!.*/bi{1}n/sh") and not grep(r"^#!/bi{1}n/sh")'
+  $ testrepohg files 'set:grep(r"^#!.*/bi{1}n/sh") and not grep(r"^#!/bi{1}n/sh")'
   [1]
diff --git a/tests/test-contrib-perf.t b/tests/test-contrib-perf.t
--- a/tests/test-contrib-perf.t
+++ b/tests/test-contrib-perf.t
@@ -170,6 +170,6 @@ 
 
   $ cd "$TESTDIR/.."
 
-  $ (syshg files -r 1.2 glob:mercurial/*.c glob:mercurial/*.py;
-  >  syshg files -r tip glob:mercurial/*.c glob:mercurial/*.py) |
+  $ (testrepohg files -r 1.2 glob:mercurial/*.c glob:mercurial/*.py;
+  >  testrepohg files -r tip glob:mercurial/*.c glob:mercurial/*.py) |
   > "$TESTDIR"/check-perf-code.py contrib/perf.py
diff --git a/tests/test-debian-packages.t b/tests/test-debian-packages.t
--- a/tests/test-debian-packages.t
+++ b/tests/test-debian-packages.t
@@ -1,7 +1,7 @@ 
 #require test-repo slow debhelper
 
   $ . "$TESTDIR/helpers-testrepo.sh"
-  $ syshgenv
+  $ testrepohgenv
 
 Ensure debuild doesn't run the testsuite, as that could get silly.
   $ DEB_BUILD_OPTIONS=nocheck
diff --git a/tests/test-docker-packaging.t b/tests/test-docker-packaging.t
--- a/tests/test-docker-packaging.t
+++ b/tests/test-docker-packaging.t
@@ -1,7 +1,7 @@ 
 #require test-repo slow docker
 
   $ . "$TESTDIR/helpers-testrepo.sh"
-  $ syshgenv
+  $ testrepohgenv
 
 Ensure debuild doesn't run the testsuite, as that could get silly.
   $ DEB_BUILD_OPTIONS=nocheck
diff --git a/tests/test-imports-checker.t b/tests/test-imports-checker.t
--- a/tests/test-imports-checker.t
+++ b/tests/test-imports-checker.t
@@ -1,7 +1,7 @@ 
 #require test-repo
 
   $ . "$TESTDIR/helpers-testrepo.sh"
-  $ syshgenv
+  $ testrepohgenv
   $ import_checker="$TESTDIR"/../contrib/import-checker.py
 
 Run the doctests from the import checker, and make sure
diff --git a/tests/test-install.t b/tests/test-install.t
--- a/tests/test-install.t
+++ b/tests/test-install.t
@@ -159,7 +159,7 @@ 
   >     print('  %s' % f)
   > EOF
 
-  $ (syshgenv; $PYTHON wixxml.py help )
+  $ (testrepohgenv; $PYTHON wixxml.py help )
   Not installed:
     help/common.txt
     help/hg-ssh.8.txt
@@ -168,7 +168,7 @@ 
     help/hgrc.5.txt
   Not tracked:
 
-  $ ( syshgenv; $PYTHON wixxml.py templates )
+  $ ( testrepohgenv; $PYTHON wixxml.py templates )
   Not installed:
   Not tracked:
 
diff --git a/tests/test-mac-packages.t b/tests/test-mac-packages.t
--- a/tests/test-mac-packages.t
+++ b/tests/test-mac-packages.t
@@ -1,7 +1,7 @@ 
 #require test-repo slow osx osxpackaging
 
   $ . "$TESTDIR/helpers-testrepo.sh"
-  $ syshgenv
+  $ testrepohgenv
 
   $ OUTPUTDIR="`pwd`"
   $ export OUTPUTDIR