basho / giddyup

Visual scorecard for riak_test.
http://giddyup.basho.com/
54 stars 12 forks source link

Adds coverage report generation and serving #141

Closed lordnull closed 10 years ago

lordnull commented 10 years ago

This takes the initial coverage report script and integrates it into giddy up proper.

Running make will generate an escript similar in purpose to the original script. However, the new script will upload results to s3 and relies on giddyup's internal sql query features and configuration idioms. This means that as giddyup changes, required changes to the script can be done in one pull request.

The script is intended to be run on the same machine that runs riak_test suites, allowing giddy up to maintain it's non-reliance on an actual riak install be accessible to it. However, since it does directly sql queries against the giddy up database, this may need to change.

This also adds another web machine resource to retrieve coverage data. It uses a similar strategy as the artifacts, simply piping them from s3 to the client.

seancribbs commented 10 years ago

So, I don't understand why the escript needs access to the database. Could we not create a new resource that lists the coverdata files (with S3 urls) given a specific filter?

Then the script could live in riak_test or elsewhere and be run as part of the build process.

lordnull commented 10 years ago

There was no good reason for the direct database access aside from that's what the original script did. That's been changed.

seancribbs commented 10 years ago

Thanks, that looks much better! I run this script after a riak_test run, right?

lordnull commented 10 years ago

That's correct. I've been testing with a run of a riak_test that uploaded to a 'local' giddy up install/S3 storage.

seancribbs commented 10 years ago

Awesome, I'll give this a whirl tomorrow.

seancribbs commented 10 years ago

So, this seemed to work on the second try. The first try I just got a bunch of "Analysis includes somethingsomething..." (lost the scrollback). The second try I got a lot of these:

14:53:11.311 [error] Error in process <0.299.0> with exit value: {{badmatch,{error,enoent}},[{giddyup_coverage,write_cover_file,3,[{file,"src/giddyup_coverage.erl"},{line,345}]}]}

But then it uploaded the files for the scorecard after spewing a lot of those.

seancribbs commented 10 years ago

Also, the files it uploaded seem empty, aside from the overview index.html thing. Am I missing something obvious in the setup?

lordnull commented 10 years ago

Works as long as the tmp directory it uses is empty, and also seems to care about order (doing a scorecard w/o platform limits first or second). So, no, you're not missing anything, it seems to be a bug.

seancribbs commented 10 years ago

If that bug can be fixed, I'm cool with this merging. We'll need to add the escript to buildbot-tester and run it at the end of builds.

lordnull commented 10 years ago

Found the error and it is fixed. Think it should be squashed a bit first?

seancribbs commented 10 years ago

No, I'm not picky about that. Let me test it one more time with the fix, then we'll merge.

seancribbs commented 10 years ago

Disregard my earlier comments, I had rebuilt the modules but not the escript. It might help if you make the script target a dep of the all target in the Makefile.

seancribbs commented 10 years ago

So, the script works correctly now, even if the local path is not present. So, as far as I'm concerned, the only things remaining are:

Good work!

seancribbs commented 10 years ago

Please merge this after fixing the minor typo and I'll get it deployed.

Awesome work! :smile_cat:

seancribbs commented 10 years ago

Actually I just fixed that typo. Merging.

seancribbs commented 10 years ago

This has been deployed to all Giddyup servers. :smiley: