Summary: | check-webkit-style: fails silently on non-existent files | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Jerdonek <cjerdonek> | ||||
Component: | Tools / Tests | Assignee: | Chris Jerdonek <cjerdonek> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cjerdonek, hamaji | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
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) Created attachment 52422 [details]
Proposed patch
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! (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! Comment on attachment 52422 [details] Proposed patch Committed: http://trac.webkit.org/changeset/57048 |
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.