commitqueuetasks_unittest does not construct FailingTestCommitQueue objects properly in its test cases

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jake Nielsen]

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.

[Jake Nielsen]

Created attachment 238982 [details]
Fixes the bug by nesting test failure strings into their own lists.

[Daniel Bates]

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]

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 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 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]

All reviewed patches have been landed.  Closing bug.