Bug 187916 - Add test-lldb-webkit
Summary: Add test-lldb-webkit
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks: 189205 189206
  Show dependency treegraph
 
Reported: 2018-07-23 11:38 PDT by Daniel Bates
Modified: 2018-11-26 16:00 PST (History)
8 users (show)

See Also:


Attachments
Patch (48.14 KB, patch)
2018-08-31 12:41 PDT, Daniel Bates
dbates: review?
Details | Formatted Diff | Diff
Patch (13.00 KB, patch)
2018-11-26 15:26 PST, Jonathan Bedard
dbates: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2018-07-23 11:38:37 PDT
We should extract the logic to build and run the lldbwebkit tests from test-webkitpy to a new script, say test-lldbwebkit, to avoid mixing the lldbwebkit tests with the webkitpy tests.
Comment 1 Alexey Proskuryakov 2018-07-27 10:19:32 PDT
One of the reasons is that we’d need to run these tests for WebCore changes, while webkitpy tests only need to run for webkitpy changes.

Running tests as part of test-webkitpy also creates difficulties in settings where we dont’t have a WebKit build in the first place.
Comment 2 Daniel Bates 2018-08-31 12:41:44 PDT
Created attachment 348663 [details]
Patch

I will teach the EWS and build.webkit.org build bots how to run test-lldb-webkit in a subsequent patch/bug.
Comment 3 Daniel Bates 2018-08-31 13:04:21 PDT
Filed bug #189205 and bug #189206 to teach the build bot and EWS how to run the LLDB WebKit tests.
Comment 4 Daniel Bates 2018-11-05 20:05:18 PST
(In reply to Daniel Bates from comment #2)
> Created attachment 348663 [details]
> Patch
> 
> I will teach the EWS and build.webkit.org build bots how to run
> test-lldb-webkit in a subsequent patch/bug.

I don’t have time at the moment to do this. Maybe someone else can?
Comment 5 Radar WebKit Bug Importer 2018-11-26 15:23:04 PST
<rdar://problem/46258959>
Comment 6 Jonathan Bedard 2018-11-26 15:26:00 PST
Created attachment 355684 [details]
Patch
Comment 7 Daniel Bates 2018-11-26 15:32:47 PST
(In reply to Jonathan Bedard from comment #6)
> Created attachment 355684 [details]
> Patch

How is this different than my patch?
Comment 8 Jonathan Bedard 2018-11-26 15:38:57 PST
(In reply to Daniel Bates from comment #7)
> (In reply to Jonathan Bedard from comment #6)
> > Created attachment 355684 [details]
> > Patch
> 
> How is this different than my patch?

I missed that, sorry. I saw comment 4 but not the patch.
Comment 9 Jonathan Bedard 2018-11-26 16:00:46 PST
Comment on attachment 348663 [details]
Patch

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

> Tools/Scripts/webkitpy/lldb_webkit_tests/main.py:63
> +    if os.path.isdir(lldb_python_directory):

Shouldn't we use the host object here to conform to webkitpy's idioms?

> Tools/Scripts/webkitpy/lldb_webkit_tests/main.py:95
> +

Can we add a --root option here? (matches with run-webkit-tests and run-api-tests)

> Tools/Scripts/webkitpy/lldb_webkit_tests/main.py:135
> +        self.printer.write_update("Checking autoinstalled packages ...")

We shouldn't need to do this for lldb tests

> Tools/Scripts/webkitpy/test/loader.py:32
> +class _Loader(unittest.TestLoader):

I feel like this should be class Loader(...) if we're allowing this to be imported, it shouldn't start with '_'