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

> >> 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).

I agree with the sentiment, but

(a) I don't think unit tests should be testing system compatibility or touching the system much at all, though they still detect missing / invalid python dependencies from time to time. While it is true that we have a mixed bag of tests at the moment, I hope that in the future we will eventually manage to separate the integration tests from the unit tests.

(b) In the event that we have some tests that were relevant towards actually exercising the system they would also have to be pretty sure to "above all, do no harm" if we are expecting some poor user in an already unknown broken state to run them and not have his system worse off afterwards, at the moment the tests litter rather heavily.

(c) I think this use case is much better targeted by a specific set of checks that look for system compatibility / sanity and make sure they are non-invasive. And we would write tests for those checks to make sure they work that wouldn't be run when you run the checks. And since they non-invasive we can run them on install, or any other time.

>
> > 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.

Agreed that they are the the rarity, my point was that other users are less likely to be useful candidates for running a test suite.

>
> >> 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.

A bookshelf is a bit of a poor analogy for real usage, though an excellent analogy of the mindset that generates these sorts of directory structures: bookshelves are fun to organize, geeks love to organize things. I, for example, have my bookshelf organized in order from hardest science to most fantastical fantasy and it works well for me but less well for people who just walked up to it. But again, it's not really a very good analogy for how one actually uses a code repository because it is supplementary storage rather than frequent random access (I also doubt you put a piece of cardboard over a section of the shelf that says "non-fiction" and another over a different part that says "science" and have to remember which one has a biography of Darwin)

A better one is your office, you keep the things you use often out in the open and you put the things you use less often and want out of sight in a tray or on a shelf (like a book, because you are unlikely to be using a book often). On my desk you will generally find a phone, a usb cable, my wallet, my keys and a variety of junk that could be put in a box but I would just have to take it back out of the box every couple days when I want to use it. Somewhere in the room will be drawers (what do you have in your drawers? do you even open them? I have ping pong paddles) and shelves. It's messy but if you were to attempt to find something on my desk it would be obvious how to proceed and you'd likely do it seconds.

But I digress :p I agree that it is a big project and the organization of it is a constant task, especially with the number of people involved. I agree that we need some submodules, though I doubt the current selection is optimal, but I aim to keep the time spent looking for something to a minimum and that includes the time spent scanning a list of files, nobody wants so many files in one place that visually scanning it becomes an issue.

>
> > 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_.

It was an argument against your hint/suggestion that the tests go into separate submodules, but I can go further on it ;)

Having tests outside of the nova directory increases the tests prominence in your visual environment before you push code as most programmers will instinctively cd back to the root of the directory and then `ls` when they get ready to push code or commit (perhaps a habit picked up from prolonged svn usage? `git status` also shows paths relative to your own which encourages you to return to the root directory). Increased prominence is good because people should be running their tests before submitting code :D With that in mind though, maybe we could just install a chrome extension that says "Are you writing testable code?" "Have you tested this code?" "Is there a test for that?" every few minutes while you browse Hacker News instead of coding.

>
> > 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.

Test code is code, and testing is integral to Nova, both of those are undeniable statements, but the point of my argument is that test code is very different than the code it is testing.

>
> > 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.

Agreed, was making the point that it is a lot harder for such a bug to slip in if they have no way of even pointing at the test code from the main code.

>
> > 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).

This again is where I propose that this use case is better covered by a sanity check and diagnostic tool (which would print out something useful for the user, like: "Checking for redis-server... NOT FOUND"), which I think is a good idea in general and will squelch a ton of faulty bug reports if people are encouraged (forced?) to run it prior to starting a service.

That was a lot of writing.