Bug 155814 – Add a model for parsing buildbot JSON with unit tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Perf Dashboard

[Ryosuke Niwa]

In order to improve our A/B testing infrastructure, add a new model class for parsing JSON fetched from buildbot.

[Ryosuke Niwa]

Created attachment 274788 [details]
Adds the classes

[Joseph Pecoraro]

Comment on attachment 274788 [details]
Adds the classes

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

r=me

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:14
> +        this._isInProgress = 'currentStep' in rawData;

This state never changes and is never checked. Will it eventually?

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:112
> +            }

Nit: Semicolon.

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:157
> +                assert.equal(typeof(value), 'object', `arguments should be a dictionary`);

Nit: Assert message here could be a string instead of a template string.

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:165
> +                for (let part of value)
> +                    assert.equal(typeof(part), 'string', 'test should be an array of strings')

You could assert value.every((function (part) { return typeof part == 'string'; }). Whatever suits your style.

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:206
> +                assert(false, `Unrecognized named arguemnt ${keys[0]}`);

Typo: "arguemnt"

> Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js:3
> +let assert = require('assert');

I see you are starting to use `let`!

[Ryosuke Niwa]

Committed r198614: <http://trac.webkit.org/changeset/198614>