[Webkit-unassigned] [Bug 152910] Add QUnit for unit testing and add a unit test to test BuildbotQueueView._appendPendingRevisionCount.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 11 12:58:57 PST 2016


Daniel Bates <dbates at webkit.org> changed:

           What    |Removed                     |Added
 Attachment #268561|commit-queue?               |commit-queue-
              Flags|                            |

--- Comment #4 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 268561
  --> https://bugs.webkit.org/attachment.cgi?id=268561

View in context: https://bugs.webkit.org/attachment.cgi?id=268561&action=review

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:2
> +QUnit.test("BuildBotQueue Test", function( assert )

Please remove the space characters inside the parentheses.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:5
> +    assert.ok(trac, "trac is not null");

What is the purpose of this test? I mean, this test is not meaningful given that trac is guaranteed to be non-null by definition of the new operator.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:11
> +            name: "Webkit Repo",

Nit: Webkit => WebKit

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:15
> +    assert.ok(queue, "queue is not null");

Similarly, this test is not meaningful by the same reason as given in my remark for line 5. Moreover, if queue was null then this script would cause a JavaScript TypeError when we assign property branches to it on line 8; => we would never execute this line.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:18
> +    assert.ok(view, "view is not null");

Similarly, this is test is not meaningful.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/tests.js:22
> +    var revisionsBehind = view.element.getElementsByClassName("message")[0].innerHTML.match(/.*(\d+) revision(|s) behind/)[1];
> +    assert.equal(revisionsBehind, 1, "assert revisions behind");

Can you elaborate on how we are one revision behind?

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160111/f9cfc8ee/attachment-0001.html>

More information about the webkit-unassigned mailing list