Code Review for September 4 from Australia

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

(forwarded from Andrew who summarized what the other group discussed)

Here's what we found today. I've left off the list so we don't prejudice
anyone who hasn't yet reviewed.

- - State is not a very meaningful name. One possible alternative that
came up was BootstrapState.
- - There's 0% code coverage for the StateInfo function (it may be tested
elsewhere, but not in the provider package).
- - There are some error code paths that are not tested (no tests for
unmarshalling failures, or for storage failures other than IsNotExist).
- - If there were a test for StateInfo, we'd get noisy output - there's no
embedded LoggingSuite in StateSuite.
- - LoadState and LoadStateFromURL are not consistent with the errors
returned: LoadState checks for boostrap-state existence and returns
a NotBootstrappedError; LoadStateFromURL does no such thing. This may be
too difficult to do, though (which HTTP return codes mean that the
environment is not bootstrapped?)
- - In state_test.go, there's a function makeDummyStorage which gets
called in most of the tests. Instead, we should use SetUp/TearDown
methods on the suite, and split out the tests that don't care about
storage into a separate suite.

Tim thought it might be useful to pass on how I came up with the
coverage analysis. I wrote a tool a while ago called gocov:
http://github.com/axw/gocov. If you run "gocov test" you'll get a JSON
file, which you can feed back into another gocov command (report or
annotate).

Example report: http://paste.ubuntu.com/6061352/
Example source annotation: http://paste.ubuntu.com/6061360/

There are third-party tools for generating prettier output: HTML,
Cobertura XML for importing into Jenkins, and coveralls.io
<http://coveralls.io>.

The "go" tool itself is going to have support for coverage analysis in
the 1.2 release (I think). It does essentially the same thing, though I
believe it may be a bit faster.

Cheers,
Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlInMhgACgkQJdeBCYSNAAOb1QCgvo8mmy0RhvLBbViEzCGcZ5+L
ylAAoKMR6AQiP11j0gFHS1ZAQ4mwLrqY
=IE1W
-----END PGP SIGNATURE-----