[Webkit-unassigned] [Bug 168994] Bots should run the dashboard tests

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=168994

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

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

If we chose to run the dashboard test separate from the rest of the Webkit tests then we should upload the dashboard test results to the Buildbot master and generate a link to the results. This output is easy to human friendly and makes it straightforward to understand the reason for the test failure. To be clear, the generated hyperlink I am referring to is the "view results" hyperlink under the step uploaded results on a Buildbot build page. You can see an example of this at <https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK1%20%28Tests%29/builds/14159>. To do this, we will likely need to modify or mimic the steps ArchiveTestResults, UploadTestResults, and ExtractTestResults.

> Tools/ChangeLog:9
> +        We pull the `--results-directory` out so that `RunDashboardTests` can override it. This way the dashboard tests results won't
> +        overwrite the regular layout test results or get mixed in with them.

It seems reasonable to separate the dashboard tests from the WebKit test. Out of curiosity, would it be terrible if they are mixed in? One benefit is that we can use the existing machinery to upload the test results and link to them.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:346
> +        self.setCommand(self.command + self._results_directory_arguments())

This is OK as-is. We can simplify this patch and avoid the need to add the function _results_directory_arguments() and override it by making use of class variables. I would define a class variable called resultDirectory := "layout-test-results" in class RunWebKitTests and override this class variable in the derived class (as we do for descriptionDone) to be "dashboard-layout-test-results". Then we can update this line to read:

self.setCommand(self.command +["--results-directory", self.resultDirectory])

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:362
> +    def _results_directory_arguments(self):
> +        return ["--results-directory", "layout-test-results"]
> +
>      def _strip_python_logging_prefix(self, line):

We can avoid the need to add this function by making use of a class variable. See my remark above.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:438
> +        self.setCommand(self.command + ['--layout-tests-directory', './Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests'])

I believe we prefer double quoted string literals for OpenSource Buildbot code though it seems we use both :( What style is used in the majority of this code? We should use consistent style.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:442
> +    def _results_directory_arguments(self):
> +        return ["--results-directory", "dashboard-layout-test-results"]

Ditto.

-- 
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/20170301/b1cd9968/attachment.html>