Bug 124703 – Remove the stderr_write attribute from StyleProcessorConfiguration

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[László Langó]

Remove the stderr_write attribute from this class in checker and replace its use with calls to a logging module logger.

[László Langó]

Created attachment 217531 [details]
Resolve a FIXME

[Brent Fulgham]

Comment on attachment 217531 [details]
Resolve a FIXME

Seems like all tests pass after this change.  r=me.

[WebKit Commit Bot]

Comment on attachment 217531 [details]
Resolve a FIXME

Clearing flags on attachment: 217531

Committed r159633: <http://trac.webkit.org/changeset/159633>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Dean Jackson]

This broke a bunch of the webkitpy tests.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 124726

[Brent Fulgham]

(In reply to comment #2)
> (From update of attachment 217531 [details])
> Seems like all tests pass after this change.  r=me.

Boy was I wrong!  EWS does not run tests, so I'm going to start requiring them for review.

[László Langó]

(In reply to comment #7)
> (In reply to comment #2)
> > (From update of attachment 217531 [details] [details])
> > Seems like all tests pass after this change.  r=me.
> 
> Boy was I wrong!  EWS does not run tests, so I'm going to start requiring them for review.

Sorry, i tested it before uploaded and all tests are passed in local. I'll try to figure out what's the problem.

[László Langó]

Created attachment 217660 [details]
Patch

[Peter Gal]

Comment on attachment 217660 [details]
Patch

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

> Tools/Scripts/webkitpy/style/checker_unittest.py:580
>      def setUp(self):
> -        self._error_messages = []
> -        """The messages written to _mock_stderr_write() of this class."""
> +        LoggingTestCase.setUp(self)

I don't think you need this setUp (and tearDown) method here, because you are explicitly calling the base class' setUp (/tearDown) method. If you don't implement the methods here it'll be inherited, so that's why there is no need for this methods.

Also calling the base class' method this way is not really good, we should use the super() call for such cases (http://docs.python.org/2/library/functions.html#super).

[László Langó]

(In reply to comment #10)
> (From update of attachment 217660 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=217660&action=review
> 
> > Tools/Scripts/webkitpy/style/checker_unittest.py:580
> >      def setUp(self):
> > -        self._error_messages = []
> > -        """The messages written to _mock_stderr_write() of this class."""
> > +        LoggingTestCase.setUp(self)
> 
> I don't think you need this setUp (and tearDown) method here, because you are explicitly calling the base class' setUp (/tearDown) method. If you don't implement the methods here it'll be inherited, so that's why there is no need for this methods.
> 
> Also calling the base class' method this way is not really good, we should use the super() call for such cases (http://docs.python.org/2/library/functions.html#super).

Ok, in this case there are wrong usage in more unittest (such as filereader_unittest.py)

[László Langó]

Created attachment 217673 [details]
Patch

[Peter Gal]

Comment on attachment 217673 [details]
Patch

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

> Tools/Scripts/webkitpy/style/error_handlers_unittest.py:39
> +        LoggingTestCase.setUp(self)

It would be better to use super(..) call here.

[László Langó]

Created attachment 217675 [details]
Patch

[Peter Gal]

(In reply to comment #14)
> Created an attachment (id=217675) [details]
> Patch

LGTM

[WebKit Commit Bot]

Comment on attachment 217675 [details]
Patch

Clearing flags on attachment: 217675

Committed r159977: <http://trac.webkit.org/changeset/159977>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.