Code review comment for lp:~thumper/juju-core/testing-docs

Revision history for this message
Tim Penhey (thumper) wrote :

Please take a look.

https://codereview.appspot.com/96110043/diff/1/doc/how-to-write-tests.txt
File doc/how-to-write-tests.txt (right):

https://codereview.appspot.com/96110043/diff/1/doc/how-to-write-tests.txt#newcode119
doc/how-to-write-tests.txt:119: // a pointer to that variable so we can
patch it in the tests.
On 2014/05/08 05:34:49, fwereade wrote:
> formatting

Done.

https://codereview.appspot.com/96110043/diff/1/doc/how-to-write-tests.txt#newcode151
doc/how-to-write-tests.txt:151: github.com/juju/testing, which brings in
the CleanupSute from the same.
On 2014/05/08 05:34:49, fwereade wrote:
> s/Sute/Suite/

Done.

https://codereview.appspot.com/96110043/diff/1/doc/how-to-write-tests.txt#newcode159
doc/how-to-write-tests.txt:159: JUJU_ENV, and JUJU_LOGGING_CONFIG
environment variables.
On 2014/05/08 05:34:49, fwereade wrote:
> The FakeHomeSuite should generally not be used, because it assumes a
context of
> a running (cmd/juju) client; almost all the code should avoid making
this
> assumption, because it's actually generally going to run on an API
server.

Perhaps that was the original intent, but it has been extended somewhat
to allow writing of custom files into the HOME directory, and the
isolation of the environment variables that juju cares about.

Perhaps we should split and isolate more.

https://codereview.appspot.com/96110043/diff/1/doc/how-to-write-tests.txt#newcode162
doc/how-to-write-tests.txt:162: api server.
On 2014/05/08 05:34:49, fwereade wrote:
> The JujuConnSuite is all fucked up, and should be used when you just
don't care
> about having coherent context available, you just kinda want
everything and
> don't want to think too hard. State server? Client? Both? Whooo! Who
cares!

> Yeah, we should fix this; but in the meantime I want people to know
it's at
> least somewhat bad/wrong, because then *maybe* someone will take it
upon
> themselves to extract a saner subset of the functionality and start
using that;
> as it is it's all too easy to just allow the rot to gently spread
without even
> knowing that's what you're doing.

Added a note to indicate that we aren't happy with it as it stands :-)

https://codereview.appspot.com/96110043/diff/1/doc/how-to-write-tests.txt#newcode215
doc/how-to-write-tests.txt:215: // of the test.
On 2014/05/08 05:34:49, fwereade wrote:
> Really, FWIW, I think we should always be clearing the env at the very
start of
> a test.

Is there not some part of the environment that we actually care about?

Pretty sure some expect bash.

https://codereview.appspot.com/96110043/

« Back to merge proposal