Bug 211195 – Add better handling of an uncleared bundle identifier in WebKitTestRunner

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Kate Cheney]

Calling exit(1) provides very little detail and makes debugging of this difficult.

[Kate Cheney]

<rdar://problem/62117145>

[Kate Cheney]

Created attachment 397982 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 397982 [details]
Patch

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

Thank you!

> Tools/WebKitTestRunner/TestController.cpp:644
> +        // ASSERT the bundle identifier has not already been set, since it can only be set once.

I think that it would be better wording to not restate what the code does. So I'd say "The bundle identifier can only be set once."

Maybe also add an explanation of how to resolve this, for the person hitting this. Something like this, if it's accurate: "WebContent process needs to be relaunched after every test that sets the bundle identifier."

[Kate Cheney]

Created attachment 398007 [details]
Patch for landing

[Kate Cheney]

(In reply to Alexey Proskuryakov from comment #3)
> Comment on attachment 397982 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=397982&action=review
> 
> Thank you!
> 
> > Tools/WebKitTestRunner/TestController.cpp:644
> > +        // ASSERT the bundle identifier has not already been set, since it can only be set once.
> 
> I think that it would be better wording to not restate what the code does.
> So I'd say "The bundle identifier can only be set once."
> 
> Maybe also add an explanation of how to resolve this, for the person hitting
> this. Something like this, if it's accurate: "WebContent process needs to be
> relaunched after every test that sets the bundle identifier."

Thanks! I updated the comment. No resolution should be necessary from now on, because the bundleID will get cleared between all tests. I wrote that in the comment.

[EWS]

Committed r260926: <https://trac.webkit.org/changeset/260926>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398007 [details].