[webkit-reviews] review denied: [Bug 90872] Ensure that test_result_writer.copy_file creates the correct directories : [Attachment 151649] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied  review:
Bug 90872: Ensure that test_result_writer.copy_file creates the correct
directories
https://bugs.webkit.org/show_bug.cgi?id=90872

Attachment 151649: Patch
https://bugs.webkit.org/attachment.cgi?id=151649&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=151649&action=review


>> Tools/Scripts/webkitpy/layout_tests/controllers/test_result_writer.py:270
>> +	    fs.maybe_make_directory(fs.dirname(dst_filepath))
> 
> Do we not need this in the other places in this file where we call
make_output_directory? Do write_text_file/write_binary_file create the
directory for you?
> 
> I suppose there's already a test that shows the write_binary_file codepath
works correctly.

This is basically duplicating the logic in self._make_output_directory(),
except that we're using src_filepath instead of self._test_name). We should
just modify _make_output_directory() to take the filename as input.

It's also not clear to me why the output_directory isn't already created, since
we tend to create pretty early on in most of the code paths through this class,
so I'm concerned that there's some other bug or inconsistency this is papering
over. What test was failing because of this? I'd like to see what code was
executing ...