Bug 198214 - Update messages_unittest.py after r245715
Summary: Update messages_unittest.py after r245715
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-23 22:29 PDT by youenn fablet
Modified: 2019-05-24 13:00 PDT (History)
7 users (show)

See Also:

Attachments
Patch (4.42 KB, patch)
2019-05-23 22:33 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch for landing (4.48 KB, patch)
2019-05-24 11:08 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2019-05-23 22:29:46 PDT 
Update messages_unittest.py after https://trac.webkit.org/changeset/245715 to cope with messages.py changes.
Comment 1 youenn fablet 2019-05-23 22:33:25 PDT 
Created attachment 370563 [details]
Patch
Comment 2 Alexey Proskuryakov 2019-05-24 09:58:19 PDT 
Comment on attachment 370563 [details]
Patch

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

> Source/WebKit/ChangeLog:3
> +        Update messages_unittest.py after https://trac.webkit.org/changeset/245715

Probably enough to say r245715 here.

> Source/WebKit/ChangeLog:9
> +        Small update to messages.py so that cancelReply is called in case of decoding error.

The patch looks good overall, but the ChangeLog confuses me. Isn't the change to messages.py more important? I know that we don't expect this path to be taken, but it has some potential for changing observed behavior anyway. If that happens, everyone will be confused by the title.
Comment 3 youenn fablet 2019-05-24 10:58:53 PDT 
(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 370563 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370563&action=review
> 
> > Source/WebKit/ChangeLog:3
> > +        Update messages_unittest.py after https://trac.webkit.org/changeset/245715
> 
> Probably enough to say r245715 here.

Will do.

> > Source/WebKit/ChangeLog:9
> > +        Small update to messages.py so that cancelReply is called in case of decoding error.
> 
> The patch looks good overall, but the ChangeLog confuses me. Isn't the
> change to messages.py more important? I know that we don't expect this path
> to be taken, but it has some potential for changing observed behavior
> anyway. If that happens, everyone will be confused by the title.

Will update the changelog.
This patch does not change observed behavior though, r245715 does.

The changes here to messages.py are just to call cancelReply in case of decode error instead of calling completionHandler(IPC::AsyncReplyError<>...) as done in r245715.
Comment 4 youenn fablet 2019-05-24 11:08:04 PDT 
Created attachment 370577 [details]
Patch for landing
Comment 5 WebKit Commit Bot 2019-05-24 11:55:35 PDT 
Comment on attachment 370577 [details]
Patch for landing

Clearing flags on attachment: 370577

Committed r245749: <https://trac.webkit.org/changeset/245749>
Comment 6 WebKit Commit Bot 2019-05-24 11:55:36 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-05-24 11:56:30 PDT 
<rdar://problem/51115730>
Comment 8 Alexey Proskuryakov 2019-05-24 13:00:50 PDT 
> The changes here to messages.py are just to call cancelReply in case of decode error instead of calling completionHandler(IPC::AsyncReplyError<>...) as done in r245715.

Which is a change in behavior.