Bug 155272 – Adding layout tests for the bot watcher's dashboard QUnit tests.

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jason Marcell]

Adding layout tests for the bot watcher's dashboard QUnit tests.

[Jason Marcell]

Created attachment 273493 [details]
Patch

[Build Bot]

Comment on attachment 273493 [details]
Patch

Attachment 273493 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/950705

New failing tests:
botwatchers-dashboard/index.html

[Build Bot]

Created attachment 273505 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 273493 [details]
Patch

Attachment 273493 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/950749

New failing tests:
botwatchers-dashboard/index.html

[Build Bot]

Created attachment 273506 [details]
Archive of layout-test-results from ews107 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 273493 [details]
Patch

Attachment 273493 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/950776

New failing tests:
botwatchers-dashboard/index.html

[Build Bot]

Created attachment 273507 [details]
Archive of layout-test-results from ews101 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 273493 [details]
Patch

Attachment 273493 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/950777

New failing tests:
botwatchers-dashboard/index.html

[Build Bot]

Created attachment 273509 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5

[Jason Marcell]

Created attachment 273510 [details]
Patch

[Jason Marcell]

Created attachment 273517 [details]
Diff of index.html

[Jason Marcell]

Created attachment 273518 [details]
Diff of tests.js.

[Jason Marcell]

I attached screenshots of the diff of the files that changed.

If you have a git svn checkout, you can do the following:
git diff -M10% HEAD^

Or do this to use FileMerge:
git difftool -t opendiff -M10% HEAD^

[Alexey Proskuryakov]

Comment on attachment 273510 [details]
Patch

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

> LayoutTests/ChangeLog:9
> +        Moved unit test files from Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests to
> +        LayoutTests/botwatchers-dashboard. Moved supporting resources into a resources folder and updated index.html accordingly

I think that there is a case where this approach will not work. Following up in e-mail.

[Jason Marcell]

Created attachment 275572 [details]
Patch

[Jason Marcell]

Created attachment 275573 [details]
Diff tests.js

[Jason Marcell]

Created attachment 275574 [details]
Diff index.html

[Jason Marcell]

Created attachment 275575 [details]
Diff overview

I've included a screenshot from my Git tool to help understand which files are new and which files have just moved.

[Daniel Bates]

Comment on attachment 275572 [details]
Patch

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

> Tools/ChangeLog:31
> +        Updated the unit tests for Trac._loaded to point to the XML files that are now located in the 'resources' directory.

Nit: Trac._loaded => Trac._loaded()

(since this is a function)

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/resources/tests.js:51
> +    client.open('GET', './resources/test-fixture-trac-rss.xml', false);

Please remove the leading "./". from the file path. Additionally, we should take this opportunity to fix the quoting style on this line to use double quoted string literals since we need to modify it anyway. Making this change will make the quoting style uses in this line consistent with the quoting style used throughout this file.

> Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests/resources/tests.js:68
> +    client.open("GET", "./resources/test-fixture-git-trac-rss.xml", false);

Please remove the leading "./" from the file path.

> Tools/Scripts/run-dashboard-tests:1
> +#! /bin/sh

Please remove the space character between the "!" and "/".

> Tools/Scripts/run-dashboard-tests:26
> +RUN_WEBKIT_TESTS="$(dirname $0)/run-webkit-tests"

We should extract $(dirname $0) into a variable, say TOOLS_DIRECTORY, and then reference this variable in the definition of RUN_WEBKIT_TESTS and TEST_DIRECTORY instead of duplicating code.

> Tools/Scripts/run-dashboard-tests:29
> +$RUN_WEBKIT_TESTS --layout-tests-directory=$TEST_DIRECTORY $*

Run `run-dashboard-tests --results-directory "my results"`

Then this script will invoke:

run-webkit-tests --layout-tests-directory=$TEST_DIRECTORY --results-directory my results

Notice that the single argument "my results" will be split into two arguments that are passed to run-webkit-tests.

From reading <https://www.gnu.org/software/bash/manual/bashref.html#Special-Parameters>, we should substitute "$@" (with quotes) for $* so as to avoid splitting and expansion of the arguments passed to this script.

[Daniel Bates]

Comment on attachment 275572 [details]
Patch

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

> Tools/ChangeLog:32
> +        (QUnit.done): Added. Removes machine-specific output from test results and calls notifyDone to let the layout test harness know that all testing is done.

Nit: notifyDone => testRunner.notifyDone()

[Jason Marcell]

Landed: http://trac.webkit.org/changeset/199180