Bug 142243 – [iOS] Crash logs can't be found on ios-simulator because WebKitTestRunner returns the wrong process name

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

Crash logs can't be found when running tests on the ios-simulator platform because WebKitTestRunner returns the wrong process name.

[David Kilzer (:ddkilzer)]

Created attachment 247810 [details]
Patch v1

[Alexey Proskuryakov]

Comment on attachment 247810 [details]
Patch v1

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

> Tools/WebKitTestRunner/TestController.cpp:698
> +#if PLATFORM(COCOA)
>      return "com.apple.WebKit.WebContent.Development";

Do we care about this being correct on the device? I think that there, it wouldn't have the Development suffix.

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> Comment on attachment 247810 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247810&action=review
> 
> > Tools/WebKitTestRunner/TestController.cpp:698
> > +#if PLATFORM(COCOA)
> >      return "com.apple.WebKit.WebContent.Development";
> 
> Do we care about this being correct on the device? I think that there, it
> wouldn't have the Development suffix.

We care when we start running tests on the device.  Why wouldn't the roots we install on the device have the .Development prefix as well?

[David Kilzer (:ddkilzer)]

(In reply to comment #0)
> Crash logs can't be found when running tests on the ios-simulator platform
> because WebKitTestRunner returns the wrong process name.

BTW, this only fixes WK2 tests.  I think I know why WK1 tests fail (we're searching for crashes using 'DumpRenderTree.app' as the process name instead of just 'DumpRenderTree'), but that's a separate fix in Python code.

Also, while testing this patch, I noticed that sometimes the wrong test would be marked as crashing, so there still may be situations where we're not getting crash logs, but that's a different bug.

[Tim Horton]

(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 247810 [details]
> > Patch v1
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=247810&action=review
> > 
> > > Tools/WebKitTestRunner/TestController.cpp:698
> > > +#if PLATFORM(COCOA)
> > >      return "com.apple.WebKit.WebContent.Development";
> > 
> > Do we care about this being correct on the device? I think that there, it
> > wouldn't have the Development suffix.
> 
> We care when we start running tests on the device.  Why wouldn't the roots
> we install on the device have the .Development prefix as well?

Because they get installed into the system location, which how we determine which variant to run (as far as I remember).

[Tim Horton]

(In reply to comment #5)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Comment on attachment 247810 [details]
> > > Patch v1
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=247810&action=review
> > > 
> > > > Tools/WebKitTestRunner/TestController.cpp:698
> > > > +#if PLATFORM(COCOA)
> > > >      return "com.apple.WebKit.WebContent.Development";
> > > 
> > > Do we care about this being correct on the device? I think that there, it
> > > wouldn't have the Development suffix.
> > 
> > We care when we start running tests on the device.  Why wouldn't the roots
> > we install on the device have the .Development prefix as well?
> 
> Because they get installed into the system location, which how we determine
> which variant to run (as far as I remember).

See:

bool isWebKitDevelopmentBuild = ![[[[NSBundle bundleWithIdentifier:@"com.apple.WebKit"] bundlePath] stringByDeletingLastPathComponent] hasPrefix:systemDirectoryPath()];

[David Kilzer (:ddkilzer)]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > (In reply to comment #2)
> > > > Comment on attachment 247810 [details]
> > > > Patch v1
> > > > 
> > > > View in context:
> > > > https://bugs.webkit.org/attachment.cgi?id=247810&action=review
> > > > 
> > > > > Tools/WebKitTestRunner/TestController.cpp:698
> > > > > +#if PLATFORM(COCOA)
> > > > >      return "com.apple.WebKit.WebContent.Development";
> > > > 
> > > > Do we care about this being correct on the device? I think that there, it
> > > > wouldn't have the Development suffix.
> > > 
> > > We care when we start running tests on the device.  Why wouldn't the roots
> > > we install on the device have the .Development prefix as well?
> > 
> > Because they get installed into the system location, which how we determine
> > which variant to run (as far as I remember).
> 
> See:
> 
> bool isWebKitDevelopmentBuild = ![[[[NSBundle bundleWithIdentifier:@"com.apple.WebKit"] bundlePath] stringByDeletingLastPathComponent] hasPrefix:systemDirectoryPath()];

Huh.  That seems weirdly arbitrary.  I figured we would have done something like bake .Development into all engineering builds, and only omit the .Development suffix on Production builds.

Since we're going to have to make lots of changes to support device testing anyway (like looking for crash logs on device or in a different directory), I don't think that's a big concern at this time.

[WebKit Commit Bot]

Comment on attachment 247810 [details]
Patch v1

Clearing flags on attachment: 247810

Committed r180983: <http://trac.webkit.org/changeset/180983>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.