It would be nice to avoid generating these useless files. Furthermore, I think we should print a warning message if wdiff is not installed, otherwise folks will likely not notice this useful feature at all (on Linux it's usually not installed by default).
It's not supposed to generate the files if wdiff isn't installed, so you've found a bug :).
Created attachment 148500 [details] Patch
Comment on attachment 148500 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148500&action=review Is there a test for this? Do we need to update the code that generates results.html to make sure it doesn't include links to the non-existent diffs? We should have a test for that. > Tools/Scripts/webkitpy/layout_tests/port/base.py:239 > + def _log_wdiff_missing(self): > + _log.warning("wdiff is not installed; please install it to generate word-by-word diffs.") > + _log.warning('') I would change this method to just return the string to output. Then you don't have to duplicate the calls to _log.warning and the calls to _log.warning are next to the 'if logging' check.
(In reply to comment #3) > (From update of attachment 148500 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=148500&action=review > > Is there a test for this? > No ... I realized after I uploaded this last night that I needed to add some tests. Will do so. > Do we need to update the code that generates results.html to make sure it doesn't include links to the non-existent diffs? We should have a test for that. > No, the code already handles that, but I'll add tests. > > Tools/Scripts/webkitpy/layout_tests/port/base.py:239 > > + def _log_wdiff_missing(self): > > + _log.warning("wdiff is not installed; please install it to generate word-by-word diffs.") > > + _log.warning('') > > I would change this method to just return the string to output. Then you don't have to duplicate the calls to _log.warning and the calls to _log.warning are next to the 'if logging' check. Ok.
Created attachment 148652 [details] update w/ review feedback
Committed r120865: <http://trac.webkit.org/changeset/120865>