Bug 37667 - Active notifications tests where implemented
Summary: Active notifications tests where implemented
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-15 12:51 PDT by John Gregg
Modified: 2010-04-21 16:16 PDT (History)
0 users

See Also:


Attachments
Patch (23.21 KB, patch)
2010-04-15 13:50 PDT, John Gregg
no flags Details | Formatted Diff | Diff
Patch (23.85 KB, patch)
2010-04-19 11:29 PDT, John Gregg
levin: review+
levin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Gregg 2010-04-15 12:51:18 PDT
Active notifications tests where implemented
Comment 1 John Gregg 2010-04-15 13:50:32 PDT
Created attachment 53470 [details]
Patch
Comment 2 David Levin 2010-04-19 11:01:03 PDT
Comment on attachment 53470 [details]
Patch

It looks like you are changing some file names basically but you didn't use "svn mv" which is preferred for a number of reasons (including history preservation and better diff's (I think)).

So r- due to lack of use of svn mv.
Comment 3 John Gregg 2010-04-19 11:29:10 PDT
Created attachment 53693 [details]
Patch
Comment 4 John Gregg 2010-04-19 11:31:37 PDT
Thought i had used svn mv the first time; in any case I reverted and did it again with svn mv, although it still created a bulky diff.

johnnyg-macbookpro:notifications johnnyg$ svn mv notifications-check-permission.html-disabled notifications-check-permission.html
A         notifications-check-permission.html
D         notifications-check-permission.html-disabled
johnnyg-macbookpro:notifications johnnyg$ svn mv notifications-display-close-events.html-disabled notifications-display-close-events.html
A         notifications-display-close-events.html
D         notifications-display-close-events.html-disabled
johnnyg-macbookpro:notifications johnnyg$ svn mv notifications-double-show.html-disabled notifications-double-show.html
A         notifications-double-show.html
D         notifications-double-show.html-disabled
johnnyg-macbookpro:notifications johnnyg$ svn mv notifications-no-icon.html-disabled notifications-no-icon.html
A         notifications-no-icon.html
D         notifications-no-icon.html-disabled
svjohnnyg-macbookpro:notifications johnnyg$ svn mv notifications-request-permission.html-disabled notifications-request-permission.html
A         notifications-request-permission.html
D         notifications-request-permission.html-disabled
svn johnnyg-macbookpro:notifications johnnyg$ svn mv notifications-with-permission.html-disabled notifications-with-permission.html
A         notifications-with-permission.html
D         notifications-with-permission.html-disabled
johnnyg-macbookpro:notifications johnnyg$ svn mv notifications-without-permission.html-disabled notifications-without-permission.html
A         notifications-without-permission.html
D         notifications-without-permission.html-disabled
johnnyg-macbookpro:notifications johnnyg$ svn status
D       notifications-with-permission.html-disabled
D       notifications-no-icon.html-disabled
D       notifications-check-permission.html-disabled
A  +    notifications-display-close-events.html
A  +    notifications-request-permission.html
D       notifications-display-close-events.html-disabled
D       notifications-request-permission.html-disabled
A  +    notifications-double-show.html
A  +    notifications-without-permission.html
D       notifications-double-show.html-disabled
D       notifications-without-permission.html-disabled
A  +    notifications-with-permission.html
A  +    notifications-no-icon.html
A  +    notifications-check-permission.html
Comment 5 David Levin 2010-04-19 13:35:32 PDT
Comment on attachment 53693 [details]
Patch

> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog	(revision 57659)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,29 @@
> +2010-04-15  John Gregg  <johnnyg@google.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Enable the notifications layout tests, but skip them on the ports that don't
> +        implement notifications.

Please add a bug link here.

btw, these lines are different from before " --- LayoutTests/fast/notifications/notifications-without-permission.html	(revision 57650)	(from LayoutTests/fast/notifications/notifications-without-permission.html-disabled:57650)"

I'm pretty sure there wasn't a "from" in the previous patch. This clearly indicates that you did svn mv.
Comment 6 John Gregg 2010-04-21 16:16:58 PDT
Committed r58021: <http://trac.webkit.org/changeset/58021>