Bug 137281

Summary: commitqueuetasks_unittest does not construct FailingTestCommitQueue objects properly in its test cases
Product: WebKit Reporter: Jake Nielsen <jake.nielsen.webkit>
Component: Tools / TestsAssignee: Jake Nielsen <jake.nielsen.webkit>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, glenn
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fixes the bug by nesting test failure strings into their own lists.
none
Adds an assert to ensure proper use of FailingTestCommitQueue in the future, and removes change that shouldn't be wrapped up in this patch. none

Jake Nielsen
Reported 2014-09-30 17:26:43 PDT
FailingTestCommitQueue expects to be constructed with a list of exceptions to throw, and a list of lists of tests to report as failed. Presently, two tests instead pass a list of strings as the second argument, but since strings can still be indexed into, the bug was hidden. Fix it.
Attachments
Fixes the bug by nesting test failure strings into their own lists. (3.34 KB, patch)
2014-09-30 17:32 PDT, Jake Nielsen 		no flags
DetailsFormatted DiffDiff
Adds an assert to ensure proper use of FailingTestCommitQueue in the future, and removes change that shouldn't be wrapped up in this patch. (3.60 KB, patch)
2014-10-01 09:53 PDT, Jake Nielsen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jake Nielsen
Comment 1 2014-09-30 17:32:25 PDT
Created attachment 238982 [details] Fixes the bug by nesting test failure strings into their own lists.
Daniel Bates
Comment 2 2014-09-30 21:57:50 PDT
Comment on attachment 238982 [details] Fixes the bug by nesting test failure strings into their own lists. View in context: https://bugs.webkit.org/attachment.cgi?id=238982&action=review Can we make FailingTestCommitQueue less error prone to use? One idea is to assert in FailingTestCommitQueue.test_results() that failures_for_run is an instance of a list object. Another idea is to convert failures_for_run to a list if it is an instance of a basestring (*). The latter approach would make FailingTestCommitQueue() work for a list-like data structure of strings as well as a list-like data structure of lists-like data structures and we wouldn't need to change tests test_double_flaky_test_failure() and test_red_test_failure(). (*) Note, basestring was removed In Python 3. Instead, we would need to convert failures_for_run to a list if it is an instance of str. > Tools/Scripts/webkitpy/tool/bot/commitqueuetask_unittest.py:79 > + flaky_tests = [result.test_name for result in flaky_results] What tests are affected by this change?
Jake Nielsen
Comment 3 2014-10-01 09:53:59 PDT
Created attachment 239032 [details] Adds an assert to ensure proper use of FailingTestCommitQueue in the future, and removes change that shouldn't be wrapped up in this patch.
Daniel Bates
Comment 4 2014-10-01 10:16:13 PDT
Comment on attachment 239032 [details] Adds an assert to ensure proper use of FailingTestCommitQueue in the future, and removes change that shouldn't be wrapped up in this patch. r=me
WebKit Commit Bot
Comment 5 2014-10-01 10:52:11 PDT
Comment on attachment 239032 [details] Adds an assert to ensure proper use of FailingTestCommitQueue in the future, and removes change that shouldn't be wrapped up in this patch. Clearing flags on attachment: 239032 Committed r174161: <http://trac.webkit.org/changeset/174161>
WebKit Commit Bot
Comment 6 2014-10-01 10:52:14 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.