• 
      

    Fix logic errors causing diffs to appear as empty files.

    Review Request #6765 — Created Jan. 13, 2015 and submitted

    Information

    Review Board
    release-2.0.x
    e7017b7...

    Reviewers

    Review Board 2.0 introduced support for showing entries in the diff
    viewer for newly-added empty files. It did this based on whether a file
    was new and whether it has 0 non-equals chunks.

    That logic should have been whether it has 0 chunks. That was a simple
    template change to fix.

    However, it exposed a problem where a valid change could have 0 chunks,
    once again triggering this problem. That was a result of checking the
    normalized insert/delete counts, rather than the raw insert/delete
    counts from the diff. That meant that if a change only had replaces, it
    would also appear as empty.

    This was particularly problematic in combination with a very old design
    flaw, where diffs that were based on a parent diff of a newly added file
    inherited the "new" state. If a series of files were introduced on a
    branch and then commits later in the branch were put up for review, they
    could easily show up as empty. Alternatively, if a file contained only
    'replace' changes, and was in the same real or bogus new state, it could
    also hit this case.

    Unit tests pass.

    The following scenarios were checked. Each showed the diff as an empty file
    before.

    Scenario 1: New files, interdiffs without modifications

    Created a new file and uploaded the diff. Then posted another diff of that
    file without modifications.

    When viewing the interdiff between the two, the diff no longer showed up as
    an empty file.

    Repeated this with a diff containing a parent diff from a newly-added commit,
    which transferred the "new" status (from an old design flaw). This failed
    before, but worked after.

    Scenario 2: New diffs, interdiffs with replace lines only

    Repeated the above scenario, but made changes to a single line between the
    revisions, generating "replace" chunks. Before, this caused the diff to appear
    as empty. Now, it shows the replaced lines.

    Scenario 3: Diff with new parent diff and replace lines only

    Created a commit introducing a file. Changed a line in the file and posted it.
    Turned off showing extra whitespace and viewed it. Saw the empty file.

    Description From Last Updated

    redefinition of unused 'FileDiffMigrationTests' from line 672

    reviewbotreviewbot
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/testing/testcase.py
          reviewboard/diffviewer/tests.py
          reviewboard/diffviewer/chunk_generator.py
      
      Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/testing/testcase.py
          reviewboard/diffviewer/tests.py
          reviewboard/diffviewer/chunk_generator.py
      
      Ignored Files:
          reviewboard/templates/diffviewer/diff_file_fragment.html
      
      
    2. reviewboard/diffviewer/tests.py (Diff revision 1)
       
       
      Show all issues
       redefinition of unused 'FileDiffMigrationTests' from line 672
      
    3. 
        
    anselina
    1. Ship It!
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (6d2a0b8)