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: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (6d2a0b8)
Loading...