handle test names with question marks in _make_output_directory

Bug: 985608
Change-Id: I5f46486511ce60fda2d34a962f115069eb2b4191
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1711097Reviewed-by: Robert Ma <robertma@chromium.org>
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#679987}

third_party/blink/tools/blinkpy/web_tests/controllers/test_result_writer.py

@@ -119,16 +119,11 @@ class TestResultWriter(object):
         self._root_output_dir = root_output_dir
         self._test_name = test_name
 
-    def _make_output_directory(self):
-        """Creates the output directory (if needed) for a given test filename."""
-        fs = self._filesystem
-        output_filename = fs.join(self._root_output_dir, self._test_name)
-        fs.maybe_make_directory(fs.dirname(output_filename))
-
     def _write_file(self, path, contents):
         if contents is not None:
-            self._make_output_directory()
-            self._filesystem.write_binary_file(path, contents)
+            fs = self._filesystem
+            fs.maybe_make_directory(fs.dirname(path))
+            fs.write_binary_file(path, contents)
 
     def _output_filename(self, suffix, extension):
         """Returns a filename based on the given suffix and extension.

third_party/blink/tools/blinkpy/web_tests/controllers/test_result_writer_unittest.py

@@ -39,54 +39,67 @@ from blinkpy.web_tests.models import test_failures
 
 class TestResultWriterTests(unittest.TestCase):
 
-    def run_test(self, failures=None, files=None):
+    def run_test(self, failures=None, files=None, filename='foo.html'):
         failures = failures or []
         host = MockSystemHost()
         host.filesystem.files = files or {}
         port = TestPort(host=host, port_name='test-mac-mac10.11', options=optparse.Values())
         actual_output = DriverOutput(text='', image=None, image_hash=None, audio=None)
         expected_output = DriverOutput(text='', image=None, image_hash=None, audio=None)
-        write_test_result(host.filesystem, port, '/tmp', 'foo.html', actual_output, expected_output, failures)
-        return host.filesystem.written_files
+        write_test_result(host.filesystem, port, '/tmp', filename, actual_output, expected_output, failures)
+        return host.filesystem
 
     def test_success(self):
         # Nothing is written when the test passes.
-        written_files = self.run_test(failures=[])
-        self.assertEqual(written_files, {})
+        fs = self.run_test(failures=[])
+        self.assertEqual(fs.written_files, {})
 
     def test_reference_exists(self):
         failure = test_failures.FailureReftestMismatch()
         failure.reference_filename = '/src/exists-expected.html'
         files = {'/src/exists-expected.html': 'yup'}
-        written_files = self.run_test(failures=[failure], files=files)
-        self.assertEqual(written_files, {'/tmp/exists-expected.html': 'yup'})
+        fs = self.run_test(failures=[failure], files=files)
+        self.assertEqual(fs.written_files, {'/tmp/exists-expected.html': 'yup'})
 
         failure = test_failures.FailureReftestMismatchDidNotOccur()
         failure.reference_filename = '/src/exists-expected-mismatch.html'
         files = {'/src/exists-expected-mismatch.html': 'yup'}
-        written_files = self.run_test(failures=[failure], files=files)
-        self.assertEqual(written_files, {'/tmp/exists-expected-mismatch.html': 'yup'})
+        fs = self.run_test(failures=[failure], files=files)
+        self.assertEqual(fs.written_files, {'/tmp/exists-expected-mismatch.html': 'yup'})
 
     def test_reference_is_missing(self):
         failure = test_failures.FailureReftestMismatch()
         failure.reference_filename = 'notfound.html'
-        written_files = self.run_test(failures=[failure], files={})
-        self.assertEqual(written_files, {})
+        fs = self.run_test(failures=[failure], files={})
+        self.assertEqual(fs.written_files, {})
 
         failure = test_failures.FailureReftestMismatchDidNotOccur()
         failure.reference_filename = 'notfound.html'
-        written_files = self.run_test(failures=[failure], files={})
-        self.assertEqual(written_files, {})
+        fs = self.run_test(failures=[failure], files={})
+        self.assertEqual(fs.written_files, {})
 
     def test_reftest_image_missing(self):
         failure = test_failures.FailureReftestNoImageGenerated()
         failure.reference_filename = '/src/exists-expected.html'
         files = {'/src/exists-expected.html': 'yup'}
-        written_files = self.run_test(failures=[failure], files=files)
-        self.assertEqual(written_files, {'/tmp/exists-expected.html': 'yup'})
+        fs = self.run_test(failures=[failure], files=files)
+        self.assertEqual(fs.written_files, {'/tmp/exists-expected.html': 'yup'})
 
         failure = test_failures.FailureReftestNoReferenceImageGenerated()
         failure.reference_filename = '/src/exists-expected.html'
         files = {'/src/exists-expected.html': 'yup'}
-        written_files = self.run_test(failures=[failure], files=files)
-        self.assertEqual(written_files, {'/tmp/exists-expected.html': 'yup'})
+        fs = self.run_test(failures=[failure], files=files)
+        self.assertEqual(fs.written_files, {'/tmp/exists-expected.html': 'yup'})
+
+    def test_slash_in_test_name(self):
+        failure = test_failures.FailureTestHarnessAssertion()
+        fs = self.run_test(failures=[failure], filename='foo.html?a/b')
+        self.assertTrue('/tmp/foo_a_b-actual.txt' in fs.written_files)
+        self.assertEqual(set(fs.written_files.keys()), {
+            '/tmp/foo_a_b-actual.txt',
+            '/tmp/foo_a_b-diff.txt',
+            '/tmp/foo_a_b-expected.txt',
+            '/tmp/foo_a_b-pretty-diff.html',
+        })
+        # Should not mkdir '/tmp/foo.html?a'
+        self.assertEqual(fs.dirs, {'/', '/tmp'})