watchlist: Add cross-checks for WatchList once it is filled.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Levin]

No definition should be allowed to remain if it is unused.

No definition should be referenced if it isn't defined.

[David Levin]

Created attachment 109212 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 109212 [details]
Patch

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

drive-by-nits.

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:32
> +from webkitpy.common.memoized import memoized

Did you intend to use this?

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:140
> +        if len(definition_set):

if definition_set: will do approximately the same thing.

[David Levin]

(In reply to comment #2)
> (From update of attachment 109212 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=109212&action=review
> 
> drive-by-nits.
> 
> > Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:32
> > +from webkitpy.common.memoized import memoized
> 
> Did you intend to use this?

I'll remove that. I attempted to use it but found that I couldn't use it for a dictionary.

> 
> > Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:140
> > +        if len(definition_set):
> 
> if definition_set: will do approximately the same thing.

Yep, will change.

[Eric Seidel (no email)]

Comment on attachment 109212 [details]
Patch

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

I'm not sure I 100% follow, but I'm willing to rubber-stamp this.

> Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py:111
> +                raise Exception('A rule for definition "%s" is empty, so it should be deleted.' % complex_definition)

I wonder if you want your own Exception subclass.

[David Levin]

Committed as http://trac.webkit.org/changeset/96390