|Summary:||check-webkit-style: fails silently on non-existent files|
|Product:||WebKit||Reporter:||Chris Jerdonek <cjerdonek>|
|Component:||Tools / Tests||Assignee:||Chris Jerdonek <cjerdonek>|
|Version:||528+ (Nightly build)|
Description Chris Jerdonek 2010-04-01 06:52:57 PDT
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.
Comment 1 Chris Jerdonek 2010-04-01 06:57:17 PDT
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)
Comment 3 Shinichiro Hamaji 2010-04-03 00:06:48 PDT
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!
Comment 4 Chris Jerdonek 2010-04-03 00:16:20 PDT
(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!