[webkit-reviews] review granted: [Bug 196323] run-api-tests: Upload test results : [Attachment 366130] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 1 15:27:25 PDT 2019

Lucas Forschler <lforschler at apple.com> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 196323: run-api-tests: Upload test results

Attachment 366130: Patch


--- Comment #6 from Lucas Forschler <lforschler at apple.com> ---
Comment on attachment 366130
  --> https://bugs.webkit.org/attachment.cgi?id=366130

View in context: https://bugs.webkit.org/attachment.cgi?id=366130&action=review

>>> Tools/Scripts/webkitpy/api_tests/manager.py:214
>>> +		 self._stream.writeln('')
>> is there any reason to have two statements here instead of:
>> self._stream.writeln('Test suite failed\n') ?
> Because of this:
> class MeteredStream(object):
>     ...
>     @staticmethod
>     def _ensure_newline(txt):
>	  return txt if txt.endswith('\n') else txt + '\n'
>     ....
>     def writeln(self, txt, now=None, pid=None):
>	  self.write(self._ensure_newline(txt), now, pid)

well... that's interesting...thanks!

>>> Tools/Scripts/webkitpy/port/base.py:1666
>>> +		 scm = SCMDetector(self._filesystem,
>> how many times are we going to run through this loop? This seems like it
could be an expensive operation.
> Hopefully not many! Yes, this could be an expensive operation (we maybe
should memoize this, actually), this loop will be run for as many repositories
appleadditions associates with the test run. We will usually run through this
loop 1 or 2 times with the current code.

if we only expect 1 or 2 times, that is fine. in the future we may want to
consider memorization..

More information about the webkit-reviews mailing list