Bug 198316 – run-benchmark should report an error if the argument to --build-directory is bogus

• Status: NEW
• Component: Tools / Tests

run-benchmark should report an error if the argument to --build-directory is bogus

Created attachment 370810 [details]
Patch

rdar://27875380

[Stephanie Lewis]

Comment on attachment 370810 [details]
Patch

There are some engineers who work on and build webkit but don't necessarily build the whole stack

[Stephanie Lewis]

Comment on attachment 370810 [details]
Patch

If you want to not support that case I'm ok with that. Secondarily what you could do is check that one of the major frameworks (webkit.framework, javascriptcore.framework ... ) or Safari.app exist.  And if throw an exception if none of those exist.  That should cover the case where people make typos

[Ryosuke Niwa]

Comment on attachment 370810 [details]
Patch

Can we also check the open file of Safari to see if we've successfully loaded the framework or not?

Created attachment 370991 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 370991 [details]
Patch

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

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:43
> +                raise Exception('"{}" does not look like a build directory'.format(browser_build_path))

I think a better error message is to just say there is no framework in the specified directory.
Otherwise, people who see this error message may get confused as to why their "valid" build directory is not accepted
if they're testing something which doesn't involve building .frameworks.

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:61
> +            _log.info('Checking either Safari.app or framwork is used from "{}".'.format(browser_build_path))

It doesn't check that at all! It simply checks that there is some open file included from the build directory...

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:63
> +            assert browser_build_absolute_path in output, 'Specified build directory "{}" but none of its content is used'.format(browser_build_path)

We should probably check that join('.framework', browser_build_absolute_path) or join('.app', browser_build_absolute_path) exist instead.
Otherwise it could be loading some setting files, caches, etc...

Created attachment 371020 [details]
Patch

[EWS Watchlist]

Comment on attachment 371020 [details]
Patch

Attachment 371020 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12336293

New failing tests:
http/tests/misc/repeat-open-cancel.html
http/tests/css/filters-on-iframes-transform.html
storage/indexeddb/index-cursor.html

[EWS Watchlist]

Created attachment 371038 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Ryosuke Niwa]

Comment on attachment 371020 [details]
Patch

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

> Tools/Scripts/webkitpy/benchmark_runner/browser_driver/osx_safari_driver.py:41
> +                env = {'DYLD_FRAMEWORK_PATH': browser_build_absolute_path, 'DYLD_LIBRARY_PATH': browser_build_absolute_path, '__XPC_DYLD_FRAMEWORK_PATH': browser_build_absolute_path, '__XPC_DYLD_LIBRARY_PATH': browser_build_absolute_path}

Can we split this into multiple lines? It's really long now.