skeleton for fuzzer; contains makefile and front end server

skeleton for fuzzer; contains makefile and front end server

BUG=skia:
TBR=jcgregorio

Committed: https://skia.googlesource.com/buildbot/+/7b2bc39cb9ff71ed933558f93db2ea49e52cd9d7

[humper, 2015-02-10 20:50:09]

ptal

[jcgregorio, 2015-02-11 12:49:11]

https://codereview.chromium.org/911143003/diff/1/fuzzer/elements.html
File fuzzer/elements.html (right):

https://codereview.chromium.org/911143003/diff/1/fuzzer/elements.html#newcode5
fuzzer/elements.html:5: The full set of web components needs for skiaperf.
for skia fuzzer.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/config/config.go
File fuzzer/go/config/config.go (right):

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/config/config.go#n...
fuzzer/go/config/config.go:3: type FrontEndConfig struct {
This is already in the the 'config' package so you can drop that from all the
names.

type FrontEnd struct {...

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/config/config.go#n...
fuzzer/go/config/config.go:8: var FuzzerConfig struct {
Same here.

type Fuzzer struct {

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go
File fuzzer/go/fuzzer/main.go (right):

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:12: )
Have all imports in one import() section, put a single blank line between the
standard packages and the rest of the packages.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:32: configFilename = flag.String("config_filename",
"fuzzer.toml", "Configuration filename")
Just 'config'.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:44: path, err :=
filepath.Abs(config.FuzzerConfig.FrontEnd.ResourcePath)
This looks like it requires that the path be provided in the config file, it
should have a reasonable fallback if not specified, like loading a directory
relative to the source directory, see:

https://github.com/google/skia-buildbot/blob/master/golden/go/skiacorrectness...

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:46: glog.Fatal(err)
Always try to give errors some context:

  glog.Fatalf("Failed to get absolute path: %s", err)

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:58:
metrics.RegisterRuntimeMemStats(metrics.DefaultRegistry)
Don't do this manually, instead call common.InitWithMetrics.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:64: addr, _ := net.ResolveTCPAddr("tcp",
"skia-monitoring-b:2003")
Put the address of the graphite server in the toml config file.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:78: if err := indexTemplate.Execute(w, indexContext{});
err != nil {
Since indexContext is empty you can drop it and instead do:

Execute(w, struct{}{})

https://codereview.chromium.org/911143003/diff/1/fuzzer/res/fuzzer/config.rb
File fuzzer/res/fuzzer/config.rb (left):

https://codereview.chromium.org/911143003/diff/1/fuzzer/res/fuzzer/config.rb#...
fuzzer/res/fuzzer/config.rb:1: require 'compass/import-once/activate'
I'm not sure how much SCSS is going to be needed on top of Polymer and the CSS
auto-prefixer. Let's drop this until it's really needed.

[humper, 2015-02-11 13:17:51]

Thanks for the detailed review, much appreciated.

https://codereview.chromium.org/911143003/diff/1/fuzzer/elements.html
File fuzzer/elements.html (right):

https://codereview.chromium.org/911143003/diff/1/fuzzer/elements.html#newcode5
fuzzer/elements.html:5: The full set of web components needs for skiaperf.
On 2015/02/11 12:49:11, jcgregorio wrote:
> for skia fuzzer.

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/config/config.go
File fuzzer/go/config/config.go (right):

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/config/config.go#n...
fuzzer/go/config/config.go:3: type FrontEndConfig struct {
On 2015/02/11 12:49:11, jcgregorio wrote:
> This is already in the the 'config' package so you can drop that from all the
> names.
> 
> type FrontEnd struct {...

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/config/config.go#n...
fuzzer/go/config/config.go:8: var FuzzerConfig struct {
On 2015/02/11 12:49:11, jcgregorio wrote:
> Same here.
> 
> type Fuzzer struct {

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go
File fuzzer/go/fuzzer/main.go (right):

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:12: )
On 2015/02/11 12:49:11, jcgregorio wrote:
> Have all imports in one import() section, put a single blank line between the
> standard packages and the rest of the packages.

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:32: configFilename = flag.String("config_filename",
"fuzzer.toml", "Configuration filename")
On 2015/02/11 12:49:11, jcgregorio wrote:
> Just 'config'.

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:44: path, err :=
filepath.Abs(config.FuzzerConfig.FrontEnd.ResourcePath)
On 2015/02/11 12:49:11, jcgregorio wrote:
> This looks like it requires that the path be provided in the config file, it
> should have a reasonable fallback if not specified, like loading a directory
> relative to the source directory, see:
> 
>
https://github.com/google/skia-buildbot/blob/master/golden/go/skiacorrectness...

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:46: glog.Fatal(err)
On 2015/02/11 12:49:11, jcgregorio wrote:
> Always try to give errors some context:
> 
>   glog.Fatalf("Failed to get absolute path: %s", err)

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:58:
metrics.RegisterRuntimeMemStats(metrics.DefaultRegistry)
On 2015/02/11 12:49:11, jcgregorio wrote:
> Don't do this manually, instead call common.InitWithMetrics.

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:64: addr, _ := net.ResolveTCPAddr("tcp",
"skia-monitoring-b:2003")
On 2015/02/11 12:49:11, jcgregorio wrote:
> Put the address of the graphite server in the toml config file.

Done.

https://codereview.chromium.org/911143003/diff/1/fuzzer/go/fuzzer/main.go#new...
fuzzer/go/fuzzer/main.go:78: if err := indexTemplate.Execute(w, indexContext{});
err != nil {
On 2015/02/11 12:49:11, jcgregorio wrote:
> Since indexContext is empty you can drop it and instead do:
> 
> Execute(w, struct{}{})

It's empty for now; I'm sure there will be stuff in there eventually so I
stubbed it out.  Done anyhow :)

https://codereview.chromium.org/911143003/diff/1/fuzzer/res/fuzzer/config.rb
File fuzzer/res/fuzzer/config.rb (left):

https://codereview.chromium.org/911143003/diff/1/fuzzer/res/fuzzer/config.rb#...
fuzzer/res/fuzzer/config.rb:1: require 'compass/import-once/activate'
On 2015/02/11 12:49:11, jcgregorio wrote:
> I'm not sure how much SCSS is going to be needed on top of Polymer and the CSS
> auto-prefixer. Let's drop this until it's really needed.

OK -- I mainly use SCSS to support nesting; I find raw CSS to be insanely
verbose in places.

I don't mind dropping dependencies on external build tools, though.  Works for
me.

[jcgregorio, 2015-02-11 13:32:23]

https://codereview.chromium.org/911143003/diff/20001/fuzzer/Makefile
File fuzzer/Makefile (right):

https://codereview.chromium.org/911143003/diff/20001/fuzzer/Makefile#newcode49
fuzzer/Makefile:49: res/css/fuzzer.css: node_modules/lastupdate res/css/main.css
You should also have an entry in .gitignore for res/css/fuzzer.css since it's a
derived file.

https://codereview.chromium.org/911143003/diff/20001/fuzzer/go/fuzzer/main.go
File fuzzer/go/fuzzer/main.go (right):

https://codereview.chromium.org/911143003/diff/20001/fuzzer/go/fuzzer/main.go...
fuzzer/go/fuzzer/main.go:61: common.InitWithMetrics("fuzzer",
&config.Fuzzer.FrontEnd.GraphiteServer)
But you already called common.Init() above.

See the discussion in the other CL about adding a common.InitWithMetricsCB to
make this easier.

[humper, 2015-02-12 17:45:23]

https://codereview.chromium.org/911143003/diff/20001/fuzzer/go/fuzzer/main.go
File fuzzer/go/fuzzer/main.go (right):

https://codereview.chromium.org/911143003/diff/20001/fuzzer/go/fuzzer/main.go...
fuzzer/go/fuzzer/main.go:61: common.InitWithMetrics("fuzzer",
&config.Fuzzer.FrontEnd.GraphiteServer)
On 2015/02/11 13:32:23, jcgregorio wrote:
> But you already called common.Init() above.
> 
> See the discussion in the other CL about adding a common.InitWithMetricsCB to
> make this easier.

Right, sorry -- thought I split them apart last week in a way that this would
work.  I'll land the callback version here, and remove it from the perf CL.

[humper, 2015-02-12 18:40:47]

On 2015/02/12 17:45:23, humper wrote:
> https://codereview.chromium.org/911143003/diff/20001/fuzzer/go/fuzzer/main.go
> File fuzzer/go/fuzzer/main.go (right):
> 
>
https://codereview.chromium.org/911143003/diff/20001/fuzzer/go/fuzzer/main.go...
> fuzzer/go/fuzzer/main.go:61: common.InitWithMetrics("fuzzer",
> &config.Fuzzer.FrontEnd.GraphiteServer)
> On 2015/02/11 13:32:23, jcgregorio wrote:
> > But you already called common.Init() above.
> > 
> > See the discussion in the other CL about adding a common.InitWithMetricsCB
to
> > make this easier.
> 
> Right, sorry -- thought I split them apart last week in a way that this would
> work.  I'll land the callback version here, and remove it from the perf CL.

Okay, done

[jcgregorio, 2015-02-12 18:46:10]

lgtm

[humper, 2015-02-12 18:48:08]

The CQ bit was checked by humper@google.com

[commit-bot: I haz the power, 2015-02-12 18:48:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/911143003/60001

[commit-bot: I haz the power, 2015-02-12 18:48:27]

Committed patchset #4 (id:60001) as
https://skia.googlesource.com/buildbot/+/7b2bc39cb9ff71ed933558f93db2ea49e52c...