Patchwork D6359: test: change test's diff generation to use mdiff for nicer output

login
register
mail settings
Submitter phabricator
Date May 10, 2019, 9:17 p.m.
Message ID <differential-rev-PHID-DREV-xcllv2eh25stckhqtcft-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39999/
State New
Headers show

Comments

phabricator - May 10, 2019, 9:17 p.m.
sangeet259 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This is patch (2/2), hence must be there in the repo after 1/2, which is at https://phab.mercurial-scm.org/D6356
  
  The current diff being used by tests upon failing is not very good. Sometimes there is a lot of redundancy.
  
  To see for yourself, follow these steps:
  
  1. Download this bundle : http://bit.ly/2DuJjsS
  
    $ hg clone https://www.mercurial-scm.org/hg
    $ cd hg/
    $ hg unbundle af8efb83e1d6.hg
    $ hg up a1e70c1dbec0^
    $ g revert --all --rev a1e70c1dbec0
    $ hg revert -i tests/test-bookmarks-pushpull.t # revert only the last three hunks (press 'n' for the first 8, and 'y' for the last three)
    $ cd tests
    $ hg diff --rev a1e70c1dbec0 # nice output from Mercurial
    $ ./run-test.py test-bookmarks-pushpull.t # poor output from the test runner
  
  If you couldn't follow those steps, you can simply check the diffs here:
  
  - test's diff: https://pastebin.com/Z4LRg4vx
  
    --- /media/sangeet/Stuff/hgrepo/Octobus/MD4/tests/test-bookmarks-pushpull.t
    +++ /media/sangeet/Stuff/hgrepo/Octobus/MD4/tests/test-bookmarks-pushpull.t.b2-binary.err
    @@ -1086,8 +1086,7 @@
       pushing to $TESTTMP/issue4455-dest (glob)
       searching for changes
       no changes found
    -  pushkey-abort: prepushkey hook exited with status 1
    -  abort: exporting bookmark @ failed!
    +  abort: prepushkey hook exited with status 1
       [255]
    
     #endif
    @@ -1126,27 +1125,27 @@
       pushing to ssh://user@dummy/issue4455-dest
       searching for changes
       no changes found
    +  remote: prepushkey hook exited with status 1
    +  abort: push failed on remote
    +  [255]
    +
    +#endif
    +
    +  $ hg -R ../issue4455-dest/ bookmarks
    +  no bookmarks set
    +
    +Using http
    +----------
    +
    +#if b2-pushkey
    +  $ hg push -B @ http # bundle2+
    +  pushing to http://localhost:$HGPORT/
    +  searching for changes
    +  no changes found
       remote: pushkey-abort: prepushkey hook exited with status 1
       abort: exporting bookmark @ failed!
       [255]
    
    -#endif
    -
    -  $ hg -R ../issue4455-dest/ bookmarks
    -  no bookmarks set
    -
    -Using http
    -----------
    -
    -#if b2-pushkey
    -  $ hg push -B @ http # bundle2+
    -  pushing to http://localhost:$HGPORT/
    -  searching for changes
    -  no changes found
    -  remote: pushkey-abort: prepushkey hook exited with status 1
    -  abort: exporting bookmark @ failed!
    -  [255]
    -
       $ hg -R ../issue4455-dest/ bookmarks
       no bookmarks set
    
    @@ -1166,8 +1165,8 @@
       pushing to ssh://user@dummy/issue4455-dest
       searching for changes
       no changes found
    -  remote: pushkey-abort: prepushkey hook exited with status 1
    -  abort: exporting bookmark @ failed!
    +  remote: prepushkey hook exited with status 1
    +  abort: push failed on remote
       [255]
    
     #endif
    
    ERROR: test-bookmarks-pushpull.t (case b2-binary) output changed
    !.
    Failed test-bookmarks-pushpull.t (case b2-binary): output changed
    # Ran 2 tests, 0 skipped, 1 failed.
    python hash seed: 133193196
  
  ---
  
  - diff used by hg diff: https://pastebin.com/qanQEsiA
  
    diff -r a1e70c1dbec0 tests/test-bookmarks-pushpull.t
    --- a/tests/test-bookmarks-pushpull.t	Thu Nov 01 22:20:00 2018 +0530
    +++ b/tests/test-bookmarks-pushpull.t	Tue Oct 17 12:38:13 2017 +0200
    @@ -1086,8 +1086,7 @@
       pushing to $TESTTMP/issue4455-dest (glob)
       searching for changes
       no changes found
    -  pushkey-abort: prepushkey hook exited with status 1
    -  abort: exporting bookmark @ failed!
    +  abort: prepushkey hook exited with status 1
       [255]
    
     #endif
    @@ -1126,8 +1125,8 @@
       pushing to ssh://user@dummy/issue4455-dest
       searching for changes
       no changes found
    -  remote: pushkey-abort: prepushkey hook exited with status 1
    -  abort: exporting bookmark @ failed!
    +  remote: prepushkey hook exited with status 1
    +  abort: push failed on remote
       [255]
    
     #endif
    @@ -1166,8 +1165,8 @@
       pushing to ssh://user@dummy/issue4455-dest
       searching for changes
       no changes found
    -  remote: pushkey-abort: prepushkey hook exited with status 1
    -  abort: exporting bookmark @ failed!
    +  remote: prepushkey hook exited with status 1
    +  abort: push failed on remote
       [255]
    
     #endif
  
  Tests uses _unified_diff based on difflib.py . The output isn't great!
  
  Mercurial's diff uses mdiff which gives better diffs.
  This patch uses mdiff's unidiff function to generate the diff for the failed tests.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  tests/run-tests.py

CHANGE DETAILS




To: sangeet259, #hg-reviewers
Cc: mercurial-devel
phabricator - May 15, 2019, 4:07 p.m.
pulkit added a comment.


  The commit message has diff which confuses `hg import`.  You need to indent those diffs.
  
  Also, remove the pastebin links as they are not required since we have diffs.
  
  > Download this bundle : http://bit.ly/2DuJjsS
  
  This link asks me to login. So definitely not a good link to put in commit message.

REPOSITORY
  rHG Mercurial

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

To: sangeet259, #hg-reviewers
Cc: pulkit, mercurial-devel

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -210,6 +210,22 @@ 
 # For Windows support
 wifexited = getattr(os, "WIFEXITED", lambda x: False)
 
+_unified_diff = difflib.unified_diff
+if PYTHON3:
+    import functools
+    _unified_diff = functools.partial(difflib.diff_bytes, difflib.unified_diff)
+
+try:
+    from mercurial import mdiff
+    # mdiff.new_diff may not be present if the mercurial version installed
+    # is not up to date hence this try block will throw exception AttributeError
+    # and if Mercurial itself is not installed in the system then it will throw
+    # an ImportError
+    _diff = mdiff.new_diff
+except (ImportError, AttributeError):
+    print("Falling back to unified_diff")
+    _diff = _unified_diff
+
 # Whether to use IPv6
 def checksocketfamily(name, port=20058):
     """return true if we can listen on localhost using family=name
@@ -598,15 +614,10 @@ 
             except OSError:
                 pass
 
-_unified_diff = difflib.unified_diff
-if PYTHON3:
-    import functools
-    _unified_diff = functools.partial(difflib.diff_bytes, difflib.unified_diff)
-
 def getdiff(expected, output, ref, err):
     servefail = False
     lines = []
-    for line in _unified_diff(expected, output, ref, err):
+    for line in _diff(expected, output, ref, err):
         if line.startswith(b'+++') or line.startswith(b'---'):
             line = line.replace(b'\\', b'/')
             if line.endswith(b' \n'):