Bug 178012 - run-webkit-tests: upload test results to multiple servers
Summary: run-webkit-tests: upload test results to multiple servers
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: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-10-06 08:53 PDT by Jonathan Bedard
Modified: 2017-10-10 10:38 PDT (History)
5 users (show)

See Also:


Attachments
Patch (12.75 KB, patch)
2017-10-06 11:44 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (13.60 KB, patch)
2017-10-09 15:42 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2017-10-06 08:53:45 PDT
Currently, we only allow results to be uploaded to a single server.  There is no reason to force this limitation, and it makes developing a new database to hold results more difficult.  run-webkit-tests should accept a list of results servers so that we can send results to multiple servers.
Comment 1 Radar WebKit Bug Importer 2017-10-06 08:54:14 PDT
<rdar://problem/34856501>
Comment 2 Jonathan Bedard 2017-10-06 11:44:28 PDT
Created attachment 323034 [details]
Patch
Comment 3 Lucas Forschler 2017-10-09 14:21:56 PDT
Comment on attachment 323034 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:395
>          """Download JSON file that only contains test

Since you are adding server_index here, you should bubble it up to get_json, as an entry point for utilizing the new functionality.
Comment 4 Jonathan Bedard 2017-10-09 15:42:34 PDT
Created attachment 323233 [details]
Patch
Comment 5 Lucas Forschler 2017-10-10 08:17:46 PDT
Comment on attachment 323233 [details]
Patch

This looks good to me. Do you want to try and test it on the UAT server before pushing to production? We likely need to verify functionality for WebKit and Internal, although I expect if it works for one, it will work for the other.
Can we add a unit test which tests 0, 1, >1 results servers?
Comment 6 Jonathan Bedard 2017-10-10 08:32:05 PDT
(In reply to Lucas Forschler from comment #5)
> Comment on attachment 323233 [details]
> Patch
> 
> This looks good to me. Do you want to try and test it on the UAT server
> before pushing to production? We likely need to verify functionality for
> WebKit and Internal, although I expect if it works for one, it will work for
> the other.
> Can we add a unit test which tests 0, 1, >1 results servers?

I have tested this when running a server locally.

As for the unit tests, we should have better testing for uploading, but since we don't have unit tests for uploading at all, I don't think we should add them in this patch.  Essentially, they will need to be integration tests because in order to test uploading, we will need to start a results server.  I'll file a bug for that.
Comment 7 Jonathan Bedard 2017-10-10 08:33:29 PDT
https://bugs.webkit.org/show_bug.cgi?id=178131 tracks adding the tests.
Comment 8 WebKit Commit Bot 2017-10-10 08:58:46 PDT
Comment on attachment 323233 [details]
Patch

Clearing flags on attachment: 323233

Committed r223132: <http://trac.webkit.org/changeset/223132>
Comment 9 WebKit Commit Bot 2017-10-10 08:58:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Jonathan Bedard 2017-10-10 10:38:08 PDT
Everything seems to be happily uploading to results servers.