This is an archive of the discontinued Mercurial Phabricator instance.

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

Authored by hooper on Mar 30 2018, 8:15 PM.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

hooper created this revision.Mar 30 2018, 8:15 PM

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

hooper updated this revision to Diff 7522.Apr 2 2018, 3:33 PM
hooper added a comment.Apr 2 2018, 3:33 PM
  • 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.

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

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.

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

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

durin42 accepted this revision.Apr 11 2018, 1:18 PM

Sorry, this fell through the cracks somehow. Queued.

This revision is now accepted and ready to land.Apr 11 2018, 1:18 PM
This revision was automatically updated to reflect the committed changes.