Patchwork D3733: tests: suppress read(-1) -> '' calls in fileobjectobserver

login
register
mail settings
Submitter phabricator
Date June 14, 2018, 4:11 p.m.
Message ID <differential-rev-PHID-DREV-thnxbhw55gumc4rgiqaq-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/32140/
State Superseded
Headers show

Comments

phabricator - June 14, 2018, 4:11 p.m.
durin42 created this revision.
Herald added a reviewer: pulkit.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This stabilizes the test output of the low-level wireproto tests
  between Python 2 and 3. I don't feel great about this change, but
  otherwise we get a ton of extra read(-1) output on Python 3, and this
  feels like a more sustainable solution.
  
  Bonus: test-ssh-proto-unbundle.t now passes on Python 3.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  contrib/python3-whitelist
  mercurial/util.py
  tests/test-ssh-proto-unbundle.t
  tests/test-ssh-proto.t

CHANGE DETAILS




To: durin42, pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - June 14, 2018, 4:12 p.m.
durin42 added a subscriber: indygreg.
durin42 added a comment.


  @indygreg I'm not sure if this undermines your testing strategy at all, but if it does we can try and figure out another path forward...

REPOSITORY
  rHG Mercurial

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

To: durin42, pulkit, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - June 16, 2018, 5:35 p.m.
indygreg added a comment.


  So Python 3 is performing a lot more `read(-1)` calls than Python 2? That, uh, seems weird and might be worth investigating.
  
  Hiding the `read()` calls does undermine the tests somewhat. But it's not a deal breaker. Let's keep this commit and keep this excessive `read(-1)` issue in the back of our heads.

REPOSITORY
  rHG Mercurial

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

To: durin42, pulkit, #hg-reviewers
Cc: indygreg, mercurial-devel

Patch

diff --git a/tests/test-ssh-proto.t b/tests/test-ssh-proto.t
--- a/tests/test-ssh-proto.t
+++ b/tests/test-ssh-proto.t
@@ -1154,7 +1154,6 @@ 
   i>     hello\n
   o> readline() -> 1:
   o>     \n
-  o> read(-1) -> 0: 
   e> read(-1) -> 42:
   e>     cannot upgrade protocols multiple times\n
   e>     -\n
@@ -1246,7 +1245,6 @@ 
   i>     invalid\n
   o> readline() -> 1:
   o>     \n
-  o> read(-1) -> 0: 
   e> read(-1) -> 46:
   e>     malformed handshake protocol: missing hello\n
   e>     -\n
@@ -1266,7 +1264,6 @@ 
   i>     invalid\n
   o> readline() -> 1:
   o>     \n
-  o> read(-1) -> 0: 
   e> read(-1) -> 48:
   e>     malformed handshake protocol: missing between\n
   e>     -\n
@@ -1288,7 +1285,6 @@ 
   i>     invalid\n
   o> readline() -> 1:
   o>     \n
-  o> read(-1) -> 0: 
   e> read(-1) -> 49:
   e>     malformed handshake protocol: missing pairs 81\n
   e>     -\n
diff --git a/tests/test-ssh-proto-unbundle.t b/tests/test-ssh-proto-unbundle.t
--- a/tests/test-ssh-proto-unbundle.t
+++ b/tests/test-ssh-proto-unbundle.t
@@ -93,7 +93,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 115:
   e>     abort: incompatible Mercurial client; bundle2 required\n
   e>     (see https://www.mercurial-scm.org/wiki/IncompatibleClient)\n
@@ -144,7 +143,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 115:
   e>     abort: incompatible Mercurial client; bundle2 required\n
   e>     (see https://www.mercurial-scm.org/wiki/IncompatibleClient)\n
@@ -274,7 +272,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 196:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -331,7 +328,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 196:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -402,7 +398,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 218:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -460,7 +455,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 218:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -532,7 +526,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 202:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -589,7 +582,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 202:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -660,7 +652,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 206:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -718,7 +709,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 206:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -790,7 +780,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 232:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -850,7 +839,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 232:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -924,7 +912,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 193:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -981,7 +968,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 193:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1052,7 +1038,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 218:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1112,7 +1097,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 218:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1186,7 +1170,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 216:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1246,7 +1229,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 216:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1326,7 +1308,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 212:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1384,7 +1365,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 212:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1457,7 +1437,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 212:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1515,7 +1494,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 212:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1590,7 +1568,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 230:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1650,7 +1627,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 230:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1733,7 +1709,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 273:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1797,7 +1772,6 @@ 
   o> read(1) -> 1: 0
   result: 0
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 273:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1875,7 +1849,6 @@ 
   o> read(1) -> 1: 1
   result: 1
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 100:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -1928,7 +1901,6 @@ 
   o> read(1) -> 1: 1
   result: 1
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 100:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -2007,7 +1979,6 @@ 
   o> read(1) -> 1: 1
   result: 1
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 152:
   e>     adding changesets\n
   e>     adding manifests\n
@@ -2064,7 +2035,6 @@ 
   o> read(1) -> 1: 1
   result: 1
   remote output: 
-  o> read(-1) -> 0: 
   e> read(-1) -> 152:
   e>     adding changesets\n
   e>     adding manifests\n
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -782,6 +782,13 @@ 
         if res is None:
             res = ''
 
+        if size == -1 and res == '':
+            # Suppress pointless read(-1) calls that return
+            # nothing. These happen _a lot_ on Python 3, and there
+            # doesn't seem to be a better workaround to have matching
+            # Python 2 and 3 behavior. :(
+            return
+
         if self.logdataapis:
             self.fh.write('%s> read(%d) -> %d' % (self.name, size, len(res)))
 
diff --git a/contrib/python3-whitelist b/contrib/python3-whitelist
--- a/contrib/python3-whitelist
+++ b/contrib/python3-whitelist
@@ -448,6 +448,7 @@ 
 test-sparse.t
 test-split.t
 test-ssh-clone-r.t
+test-ssh-proto-unbundle.t
 test-ssh-proto.t
 test-sshserver.py
 test-stack.t