Bug 155272 - Adding layout tests for the bot watcher's dashboard QUnit tests.
Summary: Adding layout tests for the bot watcher's dashboard QUnit tests.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 155274
Blocks:
  Show dependency treegraph
 
Reported: 2016-03-09 15:44 PST by Jason Marcell
Modified: 2016-04-07 14:10 PDT (History)
8 users (show)

See Also:


Attachments
Patch (41.60 KB, patch)
2016-03-09 15:45 PST, Jason Marcell
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-yosemite (967.17 KB, application/zip)
2016-03-09 16:27 PST, Build Bot
no flags Details
Archive of layout-test-results from ews107 for mac-yosemite-wk2 (1.14 MB, application/zip)
2016-03-09 16:29 PST, Build Bot
no flags Details
Archive of layout-test-results from ews101 for mac-yosemite (761.12 KB, application/zip)
2016-03-09 16:29 PST, Build Bot
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (634.34 KB, application/zip)
2016-03-09 16:37 PST, Build Bot
no flags Details
Patch (77.10 KB, patch)
2016-03-09 16:45 PST, Jason Marcell
no flags Details | Formatted Diff | Diff
Diff of index.html (996.38 KB, image/png)
2016-03-09 17:32 PST, Jason Marcell
no flags Details
Diff of tests.js. (647.73 KB, image/png)
2016-03-09 17:32 PST, Jason Marcell
no flags Details
Patch (72.16 KB, patch)
2016-04-04 14:20 PDT, Jason Marcell
dbates: review+
Details | Formatted Diff | Diff
Diff tests.js (258.97 KB, image/png)
2016-04-04 14:23 PDT, Jason Marcell
no flags Details
Diff index.html (121.05 KB, image/png)
2016-04-04 14:23 PDT, Jason Marcell
no flags Details
Diff overview (117.03 KB, image/png)
2016-04-04 14:24 PDT, Jason Marcell
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jason Marcell 2016-03-09 15:44:14 PST
Adding layout tests for the bot watcher's dashboard QUnit tests.
Comment 1 Jason Marcell 2016-03-09 15:45:55 PST
Created attachment 273493 [details]
Patch
Comment 2 Build Bot 2016-03-09 16:27:03 PST
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
Comment 3 Build Bot 2016-03-09 16:27:06 PST
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
Comment 4 Build Bot 2016-03-09 16:28:58 PST
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
Comment 5 Build Bot 2016-03-09 16:29:02 PST
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
Comment 6 Build Bot 2016-03-09 16:29:49 PST
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
Comment 7 Build Bot 2016-03-09 16:29:52 PST
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
Comment 8 Build Bot 2016-03-09 16:37:19 PST
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
Comment 9 Build Bot 2016-03-09 16:37:22 PST
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
Comment 10 Jason Marcell 2016-03-09 16:45:56 PST
Created attachment 273510 [details]
Patch
Comment 11 Jason Marcell 2016-03-09 17:32:33 PST
Created attachment 273517 [details]
Diff of index.html
Comment 12 Jason Marcell 2016-03-09 17:32:56 PST
Created attachment 273518 [details]
Diff of tests.js.
Comment 13 Jason Marcell 2016-03-09 17:34:28 PST
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^
Comment 14 Alexey Proskuryakov 2016-03-10 15:09:46 PST
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.
Comment 15 Jason Marcell 2016-04-04 14:20:11 PDT
Created attachment 275572 [details]
Patch
Comment 16 Jason Marcell 2016-04-04 14:23:34 PDT
Created attachment 275573 [details]
Diff tests.js
Comment 17 Jason Marcell 2016-04-04 14:23:56 PDT
Created attachment 275574 [details]
Diff index.html
Comment 18 Jason Marcell 2016-04-04 14:24:55 PDT
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.
Comment 19 Daniel Bates 2016-04-06 17:43:41 PDT
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.
Comment 20 Daniel Bates 2016-04-06 17:44:37 PDT
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()
Comment 21 Jason Marcell 2016-04-07 14:10:35 PDT
Landed: http://trac.webkit.org/changeset/199180