[webkit-reviews] review granted: [Bug 177751] webkitpy.tool.steps.steps_unittest.StepsTest.test_runtests_api is flakey : [Attachment 322603] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 3 16:33:57 PDT 2017


Daniel Bates <dbates at webkit.org> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 177751: webkitpy.tool.steps.steps_unittest.StepsTest.test_runtests_api is
flakey
https://bugs.webkit.org/show_bug.cgi?id=177751

Attachment 322603: Patch

https://bugs.webkit.org/attachment.cgi?id=322603&action=review




--- Comment #17 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 322603
  --> https://bugs.webkit.org/attachment.cgi?id=322603
Patch

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

> Tools/ChangeLog:12
> +	   There are a number of unit tests in webkitpy which are flakey
because they
> +	   neglected to consider logging inside memoized functions. Since we're
capturing
> +	   logging in a block, if these memoized functions have been run, we
don't see the
> +	   logging. But if they haven't yet been run, we will see the logging.

This is complicated and abstract. We should just explain the particular problem
that is occurring in steps_unittest.py. Use my comment below as a template for
how to explain this issue.

> Tools/Scripts/webkitpy/tool/steps/steps_unittest.py:44
> +	   # Port._build_path() caches the result of a function which will use
the executive. This can
> +	   # cause flakiness because we don't know the state of the cache, so
force a value to be cached.

Maybe something like this would be better:

Port._build_path() calls Tools/Scripts/webkit-build-directory and caches the
result. When capturing output this can cause the first invocation of this
function to have additional output compared to subsequent invocations. This can
cause flakiness when test order changes. We explicitly call _build_path()
before running a unit test to avoid such flakiness between test runs.

> Tools/Scripts/webkitpy/tool/steps/steps_unittest.py:45
> +	   tool = MockTool(log_executive=True)

Is it necessary to pass True for log_executive? Can we pass False?

> Tools/Scripts/webkitpy/tool/steps/steps_unittest.py:46
> +	   tool._deprecated_port = DeprecatedPort()

This is not necessary. Please remove.


More information about the webkit-reviews mailing list