Code review comment for lp:~termie/nova/move_tests

Revision history for this message
Soren Hansen (soren) wrote :

On Tue, Jul 27, 2010 at 12:59:43PM -0000, termie wrote:
>> To elaborate a bit: Tests are code. It should be just as pretty,
>> neat, and wonderful as any other piece of code.
> I agree on the cleanliness, but that has little to do with where the
> tests live.

I agree. It was a response to Monty's comment about not having pylint
shout at him about tests if they were not inside nova/.

>> I want them to be installed so that people can run tests on the
>> installed code as well. There might be environmental things that
>> factor into the tests, so I think it's very valuable to be able to
>> tell people who report bugs to run the test suite, for instance.
> I think this is a bogus statement, I don't think anybody has ever
> convinced a 'regular' user to run a test suite

I don't think I understand why you find it so far-fetched to tell a user
to "please run nova-run-tests" if they've reported a bug that looks odd
in some way. It can help identify broken dependencies, user-modified
code (people tend to tinker with stuff if they can, and since this is
all Python, bugs are just a simple $EDITOR away).

> and I think any developer who is going to go through the trouble is
> fine downloading a package or checkout (or more likely going back to
> the package or checkout they've already downloaded and left in a
> folder somewhere) to get at the tests.

I expect people doing vcs checkouts or otherwise installing from source
will be a rarity compared to people who install from packages from their
respective Linux distro.

>> In fact, I think I'd much rather move the tests for e.g. the
>> objectstore into nova/objectstore/tests. It feels perfectly natural
>> to me that tests pertaining to nova.objectstore are in
>> nova.objectstore.tests.
> That to me feels obscene, my opinion on subdirectories/submodules is
> that they exist to hide things,

In that case, I think this conversation is going to difficult :) I treat
subdirectories much like shelves in my book case. I use both to keep
things orderly and arrange stuff according to some set of criteria. I
try to put stuff on the shelves instead of throwing them all on the
floor in front of bookcase. Sure, if the directory structure was
random, it'd be counterproductive, but if it's consistent and
predictable, it's a huge help in organising stuff.

> putting tests further down a hole only makes you forget it is there
> and makes it that much more difficult to keep track of and to get to
> when you are changing something: making things separate that have no
> need to be separate for the sake of having little bins to put
> everything in just means more bins to sort through when you need to
> find something.

I'm completely missing how this is an argument for putting the tests
_all the way outside the tree_.

> Test-code is self-similar, much more so than it is similar to, say,
> objectstore's code, and more than it is to anything else in the nova
> directory. Test code should generally look the same as other test code
> and, I think, should be thought of as an entity outside of nova
> twisting the switches and pulling the nobs to make sure all the lights
> come on at the right times.

I fundamentally disagree. I believe test code is code and should be
considered just as integral a part of Nova as any other piece of code.

> In general I don't want people thinking that the test code is part of
> the nova install, I don't want people referencing the test code from
> their code,

People shouldn't have to reference test code from their code. If they
need to, that represents a bug somewhere.

> it is stuff that should never be run on a production machine and has
> no place there.

Again (unsurprisingly by now) I disagree. I think it's perfectly
reasonable to have the tests installed on production systems. If things
start acting up (they shouldn't, but everyone knows that shit happens),
I would find it comforting to be able to run the Nova tests and
confidently be able to say that Nova itself is in good shape (or --
better yet -- have the Nova test suite give valuable information about
what part of the environment changed so that things start to act up).

« Back to merge proposal