Re: output-distance: treat non-existent files as empty string (issue 583590043 by address@hidden)

[hanwenn]

https://codereview.appspot.com/583590043/diff/571790043/scripts/build/output-distance.py
File scripts/build/output-distance.py (right):

https://codereview.appspot.com/583590043/diff/571790043/scripts/build/output-distance.py#newcode531
scripts/build/output-distance.py:531: if self.contents[0] is None !=
self.contents[1] is None:
On 2020/02/29 15:51:43, Dan Eble wrote:
> This symmetric condition no longer matches the comment immediately
below.  It
> seems that you would also avoid showing the diff when the baseline
version has a
> file that is missing in the new version.  I have mixed opinions about
that.

I adjusted the comment.

> the script fail on None.strip(), though not the best option, is still
better
> than treating this as discreetly as the intentional removal of a test
case.

I disagree. Having the script crash prevents us from forming a complete
impression of what a change does, because the crash can hide other
regression failures.
 
> My first approach would probably have been to leave this earlier stuff
alone and
> address the issue below (untested):
> 
> -   cont1 = self.contents[1].strip();
> +   cont1 = self.contents[1].strip() if self.contents[1] else ''

this change is to address the failures occurring in 
https://sourceforge.net/p/testlilyissues/issues/5785/

caused by contents[0] being None.

> These changes deserve some systematic manual testing; I hope you're
covering
> that.  If this script fails to publish differences when it should,
nobody else
> involved in the countdown process is going to notice.

this is why I am extending the if case to be symmetric. We already know
that the addition of files is handled correctly. Making a minimal change
here avoids some QA.

https://codereview.appspot.com/583590043/