[webkit-reviews] review denied: [Bug 47879] FAIL: test_reset_results (webkitpy.layout_tests.run_webkit_tests_unittest.RebaselineTest) : [Attachment 71138] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied Dirk Pranke
<dpranke at chromium.org>'s request for review:
Bug 47879: FAIL: test_reset_results
(webkitpy.layout_tests.run_webkit_tests_unittest.RebaselineTest)
https://bugs.webkit.org/show_bug.cgi?id=47879

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

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=71138&action=review

>>> WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:275
>>>	     return original_open(name, mode, encoding)
>> 
>> If we are just tossing the contents of the baselines into this in-memory
buffer, then for the case of reading them, does it make sense to ever call
original_open? Or should we do something similar to the writing case, perhaps
adding an entry to a map from name to StringIO object?
> 
> The problem is that by overwriting codecs.open, this affects anything that
attempts to write anywhere, and there are parts of the code that write to the
layout-test-results directory, and I'm not 100% sure what will happen if those
get mocked as well, since we aren't symmetrically also trying to read from the
stringio buffers.
> 
> The other patch I have (posted to bug 46776) is the full solution that does
all of the above, but it's much more involved so I don't want to try and splice
it in here. If no one is happy with the fix above, it won't kill us to just
wait for the proper variant of 46776 to land to fix the "root cause" :)

Given this I think the right thing to do is to land all of the pieces
associated with bug 46776. I have a feeling that these partial fixes won't
really fix the underlying test failure.