Bug 36957 – check-webkit-style: fails silently on non-existent files

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

[Chris Jerdonek]

check-webkit-style doesn't always give a warning on non-existent files.  For example--

> check-webkit-style non-existent_file
Total errors found: 0 in 1 files
>

We should probably check for this at the beginning of check_file().  If the file doesn't exist, we should either error out or log a warning, and not increment the file_count.

[Chris Jerdonek]

We might also want to log a warning in the case below so it's more obvious that something has gone wrong:

    # We fail when style errors are found or there are no checked files.
    sys.exit(error_count > 0 or file_count == 0)

[Chris Jerdonek]

Created attachment 52422 [details]
Proposed patch

[Shinichiro Hamaji]

Comment on attachment 52422 [details]
Proposed patch

> +        if not os_path_exists(file_path):
> +            _log.error("File does not exist: %s" % file_path)
> +            return

Does this mean we won't terminate the program? If so, I slightly prefer to raise an exception here. If a user invokes the style checker by

% check-webkit-style not_exist WebCore

the error message might be missed due to a lot of style errors in WebCore, I guess.

Otherwise, looks good. Thanks for this change!

[Chris Jerdonek]

(In reply to comment #3)
> (From update of attachment 52422 [details])
> > +        if not os_path_exists(file_path):
> > +            _log.error("File does not exist: %s" % file_path)
> > +            return
> 
> Does this mean we won't terminate the program? If so, I slightly prefer to
> raise an exception here.

I wasn't sure so I chose to be more conservative.  I'm fine with an exception and will change it.  Thanks for the review!

[Chris Jerdonek]

Comment on attachment 52422 [details]
Proposed patch

Committed:

http://trac.webkit.org/changeset/57048