Bug 131359

Summary: [GTK] Remove WebKit1 unit tests
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: WebKitGTKAssignee: Martin Robinson <mrobinson>
Status: RESOLVED FIXED    
Severity: Normal CC: bunhere, cgarcia, commit-queue, dbates, gyuyoung.kim, rakuco, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Martin Robinson 2014-04-08 07:34:46 PDT
After r166504 we no longer run WebKit1 unit tests, so we can remove the Skipped lines for them.
Comment 1 Martin Robinson 2014-04-08 07:37:29 PDT
Created attachment 228834 [details]
Patch
Comment 2 Daniel Bates 2014-04-08 07:54:02 PDT
Comment on attachment 228834 [details]
Patch

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

> Tools/ChangeLog:7
> +

For your consideration I suggest repeating or rephrasing the bug description as the description in the ChangeLog entry so as to explain the reason for these removals. Maybe something like:

Following <http://trac.webkit.org/changeset/166504> we can remove the skipped WebKit1 unit tests as we no longer run the WebKit1 unit tests by default.

I'm assuming it doesn't make sense to keep these skipped test entries even if you can explicitly run the WebKit1 unit tests.
Comment 3 Martin Robinson 2014-04-08 08:06:29 PDT
(In reply to comment #2)
> (From update of attachment 228834 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228834&action=review
> 
> > Tools/ChangeLog:7
> > +
> 
> For your consideration I suggest repeating or rephrasing the bug description as the description in the ChangeLog entry so as to explain the reason for these removals. Maybe something like:
> 
> Following <http://trac.webkit.org/changeset/166504> we can remove the skipped WebKit1 unit tests as we no longer run the WebKit1 unit tests by default.

Sure. I can add the description to the ChangeLog.
 
> I'm assuming it doesn't make sense to keep these skipped test entries even if you can explicitly run the WebKit1 unit tests.

Yes, while you can still run the unit tests manually, they will soon be removed. I think the fact that they were not removed from the unit tests script is just an oversight or Carlos wanted to do it in pieces. Perhaps it does make sense to remove them from this script only while removing them completely though. I'll rework the patch.
Comment 4 Carlos Garcia Campos 2014-04-08 08:09:25 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 228834 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=228834&action=review
> > 
> > > Tools/ChangeLog:7
> > > +
> > 
> > For your consideration I suggest repeating or rephrasing the bug description as the description in the ChangeLog entry so as to explain the reason for these removals. Maybe something like:
> > 
> > Following <http://trac.webkit.org/changeset/166504> we can remove the skipped WebKit1 unit tests as we no longer run the WebKit1 unit tests by default.
> 
> Sure. I can add the description to the ChangeLog.
> 
> > I'm assuming it doesn't make sense to keep these skipped test entries even if you can explicitly run the WebKit1 unit tests.
> 
> Yes, while you can still run the unit tests manually, they will soon be removed. I think the fact that they were not removed from the unit tests script is just an oversight or Carlos wanted to do it in pieces. Perhaps it does make sense to remove them from this script only while removing them completely though. I'll rework the patch.

I forgot to remove the skipped tests
Comment 5 Martin Robinson 2014-04-08 09:19:37 PDT
Created attachment 228842 [details]
Patch
Comment 6 Martin Robinson 2014-04-08 16:36:59 PDT
Committed r166977: <http://trac.webkit.org/changeset/166977>