Bug 90416 – webkitpy: lint code in webkitpy.layout_tests.models

• Component: New Bugs
• Status: RESOLVED FIXED

[Dirk Pranke]

webkitpy: lint code in webkitpy.layout_tests.models

[Dirk Pranke]

Created attachment 150506 [details]
Patch

[Ojan Vafai]

Comment on attachment 150506 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py:215
> +            indices[i] += 1  # pylint: disable=W0631
> +            for j in range(i + 1, r):  # pylint: disable=W0631

What is the complaint here? Can we just fix it?

> Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:405
> +        self.name = None  # this is the path in the line itself
> +        self.path = None  # this is the normpath of self.name

Can you put a FIXME to rename these? original_path and normalize_path sgtm.

> Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:115
> +        super(FailureTimeout, self).__init__()

IIRC, there's some issue with use super. I don't remember what it was, but everywhere else in webkitpy that I remember we just call the super-class directly. I think we probably should do that here for consistency sake at least.

[Dirk Pranke]

(In reply to comment #2)
> (From update of attachment 150506 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=150506&action=review
> 
> > Tools/Scripts/webkitpy/layout_tests/models/test_configuration.py:215
> > +            indices[i] += 1  # pylint: disable=W0631
> > +            for j in range(i + 1, r):  # pylint: disable=W0631
> 
> What is the complaint here? Can we just fix it?
> 

W0631 == "potentially uninitialized variable". I think this is a bug in the linter, because I don't see any way that i can be unused. I'll add a comment to that effect.

> > Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py:405
> > +        self.name = None  # this is the path in the line itself
> > +        self.path = None  # this is the normpath of self.name
> 
> Can you put a FIXME to rename these? original_path and normalize_path sgtm.
>

Sure.
 
> > Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:115
> > +        super(FailureTimeout, self).__init__()
> 
> IIRC, there's some issue with use super. I don't remember what it was, but everywhere else in webkitpy that I remember we just call the super-class directly. I think we probably should do that here for consistency sake at least.

The issue w/ super has to do with multiple inheritance and __init__() methods that take different signatures; it's hard to use super() correctly if __init__() doesn't either (a) take no arguments or (b) take (..., *args, **kwargs). 

That said, I've found that using super is somewhat less brittle than calling the superclasses directly. I think you'll find plenty of uses of both at this point, and webkitpy.layout_tests mostly uses super().

[Dirk Pranke]

Committed r122387: <http://trac.webkit.org/changeset/122387>