Bug 58101

Summary: new-run-webkit-tests: minor cleanup to be consistent in handling missing files
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: crogers, mihaip, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 57987    
Attachments:
Description Flags
Patch
none
Patch ojan: review+

Description Dirk Pranke 2011-04-07 17:17:29 PDT
new-run-webkit-tests: minor cleanup to be consistent in handling missing files
Comment 1 Dirk Pranke 2011-04-07 17:22:34 PDT
Created attachment 88741 [details]
Patch
Comment 2 Dirk Pranke 2011-04-07 19:27:09 PDT
Created attachment 88755 [details]
Patch
Comment 3 Dirk Pranke 2011-04-07 19:31:50 PDT
this patch is some cleanup needed prior to adding support for webaudio; it makes the changes in the webaudio patch easier to understand.
Comment 4 Ojan Vafai 2011-04-08 00:44:55 PDT
Comment on attachment 88755 [details]
Patch

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

> Tools/ChangeLog:10
> +        new-run-webkit-tests: clean up the way we handle missing files,
> +        to be consistent. With this change, the Port.expected_X() and
> +        Driver.run_test() routines should return None if there is no
> +        output, not ''.
> +
> +        https://bugs.webkit.org/show_bug.cgi?id=58101

Usually the ChangeLog format is:

Reviewed by ...

one-line description that usually matches the bug title
http://bugs.webkit.org/...

detailed description here

* files modified

> Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py:228
> +            self._port.compare_text(self._get_normalized_output_text(actual_text),
> +                                    # Assuming expected_text is already normalized.
> +                                    expected_text)):

This is difficult for me to read. Could you put it all on one line and put the comment above the if-statement?
Comment 5 Dirk Pranke 2011-04-08 12:34:55 PDT
(In reply to comment #4)
> (From update of attachment 88755 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=88755&action=review
> 
> > Tools/ChangeLog:10
> > +        new-run-webkit-tests: clean up the way we handle missing files,
> > +        to be consistent. With this change, the Port.expected_X() and
> > +        Driver.run_test() routines should return None if there is no
> > +        output, not ''.
> > +
> > +        https://bugs.webkit.org/show_bug.cgi?id=58101
> 
> Usually the ChangeLog format is:
> 
> Reviewed by ...
> 
> one-line description that usually matches the bug title
> http://bugs.webkit.org/...
> 
> detailed description here
> 

Okay. I'm not sure how it is that I've done hundreds of commits and am learning this just now :)

> * files modified
> 
> > Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py:228
> > +            self._port.compare_text(self._get_normalized_output_text(actual_text),
> > +                                    # Assuming expected_text is already normalized.
> > +                                    expected_text)):
> 
> This is difficult for me to read. Could you put it all on one line and put the comment above the if-statement?

Will do as part of landing this.
Comment 6 Dirk Pranke 2011-04-08 12:48:57 PDT
Committed r83327: <http://trac.webkit.org/changeset/83327>