Bug 131359 - [GTK] Remove WebKit1 unit tests
Summary: [GTK] Remove WebKit1 unit tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-08 07:34 PDT by Martin Robinson
Modified: 2014-04-08 16:38 PDT (History)
7 users (show)

See Also:


Attachments
Patch (1.71 KB, patch)
2014-04-08 07:37 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch (374.36 KB, patch)
2014-04-08 09:19 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>