Bug 156920 – Add JSC test results in JSON format to a Buildbot log

• Component: New Bugs
• Status: RESOLVED FIXED

[Srinivasan Vijayaraghavan]

Add JSC test results in json format to a buildbot log

[Srinivasan Vijayaraghavan]

Created attachment 277084 [details]
Patch

[Srinivasan Vijayaraghavan]

Will be easier to iterate on the front-end component of #156595 once this is up.

[Srinivasan Vijayaraghavan]

This addition to the back-end will make it easier to iterate on the front-end changes in #156595.

[WebKit Commit Bot]

Comment on attachment 277084 [details]
Patch

Rejecting attachment 277084 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Commit Bot]

Comment on attachment 277084 [details]
Patch

Clearing flags on attachment: 277084

Committed r199916: <http://trac.webkit.org/changeset/199916>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 156989

[Daniel Bates]

Comment on attachment 277084 [details]
Patch

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

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:302
> +    command = ["perl", "./Tools/Scripts/run-javascriptcore-tests", "--no-build", WithProperties("--%(configuration)s", "--json-output=%(_jsonFileName)s")]

This will either break buildbot or will not have the desired effect because WithProperties() only interpolates BuildBot property names and will try to substitute the value of the non-existent BuildBot property _jsonFileName for %(_jsonFileName)s. That is, WithProperties() will not interpolate the value of the local variable _jsonFileName.

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:304
> +    logfiles = {"json": _jsonFileName}

As mentioned to Srinivasan in-person today (04/25), I do not see the need to add the JSON output files as a log file that is hyperlinked from a Buildbot build page as a human is unlikely to make use of the JSON result. Moreover, we should explicitly upload the JSON output from the slave to the master (say, as part of the layout test results archive) instead of taking advantage of the implicit uploading of log files.

[Alexey Proskuryakov]

> As mentioned to Srinivasan in-person today (04/25), I do not see the need to add the JSON output files as a log file that is hyperlinked from a Buildbot build page as a human is unlikely to make use of the JSON result.

I think that it's valuable to have the link for debugging, if nothing else.

>  Moreover, we should explicitly upload the JSON output from the slave to the master (say, as part of the layout test results archive) instead of taking advantage of the implicit uploading of log files.

This is definitely not part of the layout tests archive, so we would need to devise an entirely new location for it (including a cleanup strategy etc). It seemed like an overkill to me.

[Srinivasan Vijayaraghavan]

Created attachment 277531 [details]
Patch

[Srinivasan Vijayaraghavan]

Now using Python's .format() method for interpolation.

[Srinivasan Vijayaraghavan]

Created attachment 277713 [details]
Patch

[Daniel Bates]

Comment on attachment 277713 [details]
Patch

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

r=me

> Tools/BuildSlaveSupport/build.webkit.org-config/master.cfg:302
> +    command = ["perl", "./Tools/Scripts/run-javascriptcore-tests", "--no-build", WithProperties("--%(configuration)s", "--json-output={0}".format(jsonFileName))]

This is OK as-is. We could have passed jsonFileName as an explicit argument instead of interpolating it into the --json-output argument as Getopt::Long, used by run-javascriptcore-tests to parse command line arguments, is smart enough to know that the argument that follow the flag --json-output is the value for the flag.

[WebKit Commit Bot]

Comment on attachment 277713 [details]
Patch

Clearing flags on attachment: 277713

Committed r200253: <http://trac.webkit.org/changeset/200253>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Srinivasan Vijayaraghavan]

Fix on the way!

[Srinivasan Vijayaraghavan]

Created attachment 278150 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 278150 [details]
Patch

Clearing flags on attachment: 278150

Committed r200450: <http://trac.webkit.org/changeset/200450>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.