Patchwork D2988: fix: use a portable python script instead of sed in test

login
register
mail settings
Submitter phabricator
Date March 31, 2018, 12:15 a.m.
Message ID <differential-rev-PHID-DREV-k6jzs56jfb4jatalv3ag-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30029/
State Superseded
Headers show

Comments

phabricator - March 31, 2018, 12:15 a.m.
hooper created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

AFFECTED FILES
  tests/test-fix.t

CHANGE DETAILS




To: hooper, #hg-reviewers
Cc: mercurial-devel
phabricator - March 31, 2018, 2:56 a.m.
mharbison72 added a comment.


  sys.std* need the procutil.setbinary() treatment, and globbing applied to the delta below to keep Windows happy.  Also, test-fix-topology.t probably needs the same fix.
  
    --- c:/Users/Matt/projects/hg/tests/test-fix.t
    +++ c:/Users/Matt/projects/hg/tests/test-fix.t.err
    @@ -930,7 +935,7 @@
       $ hg commit -Aqm "foo"
       $ printf "Foo\nbar\nBaz\n" > foo.changed
       $ hg --debug fix --working-dir
    -  subprocess: /usr/bin/python $TESTTMP/uppercase.py 1-1 3-3
    +  subprocess: c:/Python27/python.exe $TESTTMP/uppercase.py 1-1 3-3
    
       $ cd ..
  
  I'm not sure if you have any idea why, but it seems like {rootpath} is getting lost on Windows.  (The changes here are because the printfs don't get run properly on Windows when run directly.)
  
  https://www.mercurial-scm.org/pipermail/mercurial-devel/2018-March/114691.html

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers
Cc: mharbison72, mercurial-devel
phabricator - April 2, 2018, 7:33 p.m.
hooper added a comment.


  > - subprocess: /usr/bin/python $TESTTMP/uppercase.py 1-1 3-3 +  subprocess: c:/Python27/python.exe $TESTTMP/uppercase.py 1-1 3-3
  
  I had wondered if there's a reason we don't substitute $PYTHON like we do $TESTTMP? The glob makes the test a little weaker.
  
  > I'm not sure if you have any idea why, but it seems like {rootpath} is getting lost on Windows.  (The changes here are because the printfs don't get run properly on Windows when run directly.)
  
  Not sure.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers
Cc: mharbison72, mercurial-devel
phabricator - April 3, 2018, 2:09 a.m.
mharbison72 added a subscriber: durin42.
mharbison72 added a comment.


  I'm not sure what the state of this is (`hg phabread` failed for 3022 and 3023), but this works on Windows.  I've got a separate patch to fix the printfs (which doesn't help the {rootpath} issue I mentioned).
  
  In https://phab.mercurial-scm.org/D2988#48851, @hooper wrote:
  
  > > - subprocess: /usr/bin/python $TESTTMP/uppercase.py 1-1 3-3 +  subprocess: c:/Python27/python.exe $TESTTMP/uppercase.py 1-1 3-3
  >
  > I had wondered if there's a reason we don't substitute $PYTHON like we do $TESTTMP? The glob makes the test a little weaker.
  
  
  Not sure, other than the invocation via $PYTHON is relatively new.  I think @durin42 was the one who did most of that work.  Masking the output seems useful too.  (Hopefully it doesn't cause a headache on Windows.)
  
  >> I'm not sure if you have any idea why, but it seems like {rootpath} is getting lost on Windows.  (The changes here are because the printfs don't get run properly on Windows when run directly.)
  > 
  > Not sure.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
phabricator - April 3, 2018, 2:13 a.m.
hooper added a comment.


  In https://phab.mercurial-scm.org/D2988#48864, @mharbison72 wrote:
  
  > I'm not sure what the state of this is (`hg phabread` failed for 3022 and 3023), but this works on Windows.  I've got a separate patch to fix the printfs (which doesn't help the {rootpath} issue I mentioned).
  
  
  The other revisions were phabsend mistakes, I closed them.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
phabricator - April 3, 2018, 3:09 a.m.
durin42 added a comment.


  In https://phab.mercurial-scm.org/D2988#48864, @mharbison72 wrote:
  
  > I'm not sure what the state of this is (`hg phabread` failed for 3022 and 3023), but this works on Windows.  I've got a separate patch to fix the printfs (which doesn't help the {rootpath} issue I mentioned).
  >
  > In https://phab.mercurial-scm.org/D2988#48851, @hooper wrote:
  >
  > > > - subprocess: /usr/bin/python $TESTTMP/uppercase.py 1-1 3-3 +  subprocess: c:/Python27/python.exe $TESTTMP/uppercase.py 1-1 3-3
  > >
  > > I had wondered if there's a reason we don't substitute $PYTHON like we do $TESTTMP? The glob makes the test a little weaker.
  >
  >
  > Not sure, other than the invocation via $PYTHON is relatively new.  I think @durin42 was the one who did most of that work.  Masking the output seems useful too.  (Hopefully it doesn't cause a headache on Windows.)
  
  
  It never occurred to me. The work around $PYTHON in the tests was all around getting invocations to consistently use a single Python, instead of ninja-fallback to `python` when on `python3` or `pypy`.
  
  Probably worth doing though?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
phabricator - April 11, 2018, 3:24 a.m.
mharbison72 added a comment.


  Any movement on this?  There's a lot of test spew on Windows that this would cut out.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers
Cc: durin42, mharbison72, mercurial-devel
phabricator - April 11, 2018, 5:18 p.m.
durin42 accepted this revision.
durin42 added a comment.
This revision is now accepted and ready to land.


  Sorry, this fell through the cracks somehow. Queued.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2988

To: hooper, #hg-reviewers, durin42
Cc: durin42, mharbison72, mercurial-devel

Patch

diff --git a/tests/test-fix.t b/tests/test-fix.t
--- a/tests/test-fix.t
+++ b/tests/test-fix.t
@@ -1,3 +1,55 @@ 
+A script that implements uppercasing of specific lines in a file. This
+approximates the behavior of code formatters well enough for our tests.
+
+  $ UPPERCASEPY="$TESTTMP/uppercase.py"
+  $ cat > $UPPERCASEPY <<EOF
+  > import sys
+  > lines = set()
+  > for arg in sys.argv[1:]:
+  >   if arg == 'all':
+  >     sys.stdout.write(sys.stdin.read().upper())
+  >     sys.exit(0)
+  >   else:
+  >     first, last = arg.split('-')
+  >     lines.update(range(int(first), int(last) + 1))
+  > for i, line in enumerate(sys.stdin.readlines()):
+  >   if i + 1 in lines:
+  >     sys.stdout.write(line.upper())
+  >   else:
+  >     sys.stdout.write(line)
+  > EOF
+  $ TESTLINES="foo\nbar\nbaz\nqux\n"
+  $ printf $TESTLINES | $PYTHON $UPPERCASEPY
+  foo
+  bar
+  baz
+  qux
+  $ printf $TESTLINES | $PYTHON $UPPERCASEPY all
+  FOO
+  BAR
+  BAZ
+  QUX
+  $ printf $TESTLINES | $PYTHON $UPPERCASEPY 1-1
+  FOO
+  bar
+  baz
+  qux
+  $ printf $TESTLINES | $PYTHON $UPPERCASEPY 1-2
+  FOO
+  BAR
+  baz
+  qux
+  $ printf $TESTLINES | $PYTHON $UPPERCASEPY 2-3
+  foo
+  BAR
+  BAZ
+  qux
+  $ printf $TESTLINES | $PYTHON $UPPERCASEPY 2-2 4-4
+  foo
+  BAR
+  baz
+  QUX
+
 Set up the config with two simple fixers: one that fixes specific line ranges,
 and one that always fixes the whole file. They both "fix" files by converting
 letters to uppercase. They use different file extensions, so each test case can
@@ -10,10 +62,10 @@ 
   > evolution.createmarkers=True
   > evolution.allowunstable=True
   > [fix]
-  > uppercase-whole-file:command=sed -e 's/.*/\U&/'
+  > uppercase-whole-file:command=$PYTHON $UPPERCASEPY all
   > uppercase-whole-file:fileset=set:**.whole
-  > uppercase-changed-lines:command=sed
-  > uppercase-changed-lines:linerange=-e '{first},{last} s/.*/\U&/'
+  > uppercase-changed-lines:command=$PYTHON $UPPERCASEPY
+  > uppercase-changed-lines:linerange={first}-{last}
   > uppercase-changed-lines:fileset=set:**.changed
   > EOF
 
@@ -878,7 +930,7 @@ 
   $ hg commit -Aqm "foo"
   $ printf "Foo\nbar\nBaz\n" > foo.changed
   $ hg --debug fix --working-dir
-  subprocess: sed -e '1,1 s/.*/\U&/' -e '3,3 s/.*/\U&/'
+  subprocess: /usr/bin/python $TESTTMP/uppercase.py 1-1 3-3
 
   $ cd ..