Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl currently hardcodes the list of tests that it runs. It should read them dynamically from the 'resources' subdirectory.
Created attachment 281936 [details] Patch v1
Comment on attachment 281936 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=281936&action=review > Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl:48 > + ".pl" => "get_function_line_ranges_for_perl", > + ".py" => "get_function_line_ranges_for_python", > + ".java" => "get_function_line_ranges_for_java", > + ".cpp" => "get_function_line_ranges_for_cpp", > + ".js" => "get_function_line_ranges_for_javascript", > + ".css" => "get_selector_line_ranges_for_css", > + ".swift" => "get_function_line_ranges_for_swift", For your consideration, I suggest that we sort these lines by hash key such that the first line is for key ".cpp". > Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl:146 > + next if m/^\./; The leading 'm' is not needed when '/' is used as regular expression delimiter. > Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl:148 > + next if m/-expected.txt$/; The leading 'm' is not needed when '/' is used as regular expression delimiter. Take the current filename ($_) to be "A-expectedAtxt". Then the regular expression will match because the '.' (period) between the name of the file and its extension is not escaped and will be interpreted as a pattern metacharacter. This is unlikely to cause an issue in practice. One way to fix this issue is to escape the '.' by preceding it with a forward slash ('\') as you did on line 146. Alternatively you can disable interpretation of pattern metacharacters in the expression "-expected.txt" by placing it inside \Q...\E such that the regular expression reads /\Q-expected.txt\E$/.
Comment on attachment 281936 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=281936&action=review Thanks Dan! >> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl:48 >> + ".swift" => "get_function_line_ranges_for_swift", > > For your consideration, I suggest that we sort these lines by hash key such that the first line is for key ".cpp". Good call! Will fix. >> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl:146 >> + next if m/^\./; > > The leading 'm' is not needed when '/' is used as regular expression delimiter. Will fix. >> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/parser_unittests.pl:148 >> + next if m/-expected.txt$/; > > The leading 'm' is not needed when '/' is used as regular expression delimiter. > > Take the current filename ($_) to be "A-expectedAtxt". Then the regular expression will match because the '.' (period) between the name of the file and its extension is not escaped and will be interpreted as a pattern metacharacter. This is unlikely to cause an issue in practice. One way to fix this issue is to escape the '.' by preceding it with a forward slash ('\') as you did on line 146. Alternatively you can disable interpretation of pattern metacharacters in the expression "-expected.txt" by placing it inside \Q...\E such that the regular expression reads /\Q-expected.txt\E$/. Good catch--wrote the regex too quickly! :) Will also fix the m// operator.
Committed r202433: <http://trac.webkit.org/changeset/202433>
In final version of patch, I think next if $_ eq EXPECTED_RESULTS_SUFFIX; would be more readable then next if /\Q@{[EXPECTED_RESULTS_SUFFIX]}\E$/;
Oops, ignore this.