Merge lp:~allenap/maas-test/configure-abstract into lp:maas-test

[Gavin Panella]

Use abstract properties to make the test cases configuration clearer.

The TestMAAS.configure() call had become vestigial with the passing of the raw arguments object. I've removed it, and replaced the configurable properties with abstract properties. This way we can configure the subclass in a natural way, without extra function-calling boilerplate, but we also document the meaning of the configuration, and we will encounter errors if we forget to provide the necessary configuration.

I've also removed the prepare_parser() call as an unnecessary indirection. Argument parsers are not mutated in practice, so we need only prepare one.

I've also changed --interactive-enlistment to --interactive. Firstly it was a lot to type. More importantly it's relevant when using the daemon mode, coming in a later branch. The daemon mode will have interactive elements (it's more of a server than a "proper" daemon really). The daemon mode may be delayed - it has been de-prioritised - but I'd like to switch to this spelling of the flag now, in preparation.

[Jeroen T. Vermeulen (jtv)]

There are definitely good things here: avoiding the weird dynamic setting of a class variable, renaming --interactive-enlistment to --interactive, and the new help string for the --interactive option.

I also like having documentation for the arguments, but the abstract properties are a tail wagging its dog. Instead of defining an option in parser.py and the manpage, we'd have to define them in parser.py, cases.py, main.py and the manpage. K.I.S.S.! Forget just the abstract property and you violate the design but nothing actually happens; forget just the bit in ConfiguredTestMAAS and you get a traceback to remind you to define the abstract property; forget both and you violate the design but get a different traceback without the reminder. It's bad enough that manpages are generally like that, but in the code it's easy to avoid.

The option definitions on the arguments parser are effectively documentation in themselves, which if anything just needs to be brought to the fore. The args object encapsulates them in a way that makes them easy to identify as arguments. That's actually a pretty good setup — light, flexible, and clear. All in one place, so consistency comes naturally without enforcement. If you want to isolate and document the options that are relevant to test scenarios, I'd suggest sticking very close to the existing "args" pattern. Once you have cross-module inheritance relationships separating a field's specification, use, and definition, you're no longer any better off than with simple cross-module delegation.

Nor do I agree with inlining prepare_parser() into module-level code. A declarative list of argparse.Action as a module-level variable would make sense — though redundant if abstract properties actually serve their purpose of documenting those options! — but I fail to see any advantage in making these method calls side effects of importing the module. Without reliable policing of __all__, module-level code at zero indentation just gives me lots more items to read that may hide effective interface points for the module. Doing it this way just makes it awkward to test properties like “the parser accepts this command line specified by the original product design,” or “this particular mistake on the command line leads to this user-friendly error message.”

One other thing: main(), in main.py, has imports in unexpected places. Explain why those are necessary, or move them to the expected places.

review: Needs Fixing

[lp:~allenap/maas-test/configure-abstract]

59. By Gavin Panella on 2013-11-26

Merged refactor-prep into configure-abstract, resolving conflicts.

60. By Gavin Panella on 2013-11-26

Snapshot changed man page.

61. By Gavin Panella on 2013-11-26

Add argument group for image selection.

62. By Gavin Panella on 2013-11-26

Improve metavars.

63. By Gavin Panella on 2013-11-26

Not using testresources right now.

64. By Gavin Panella on 2013-11-26

Merged refactor-prep into configure-abstract, resolving conflicts.

65. By Gavin Panella on 2013-11-26

Merged refactor-prep into configure-abstract, resolving conflicts.

66. By Gavin Panella on 2013-11-26

Switch more options to using abstract properties.

67. By Gavin Panella on 2013-11-26

Move proxy options into their own argument group.

68. By Gavin Panella on 2013-11-26

Add some TODOs.

[Gavin Panella (allenap)]

Thanks for the review. I disagree with most of it! But let's see where
we can go from here, and if I can win you over :)

> There are definitely good things here: avoiding the weird dynamic setting of a
> class variable, renaming --interactive-enlistment to --interactive, and the
> new help string for the --interactive option.
>
> I also like having documentation for the arguments, but the abstract
> properties are a tail wagging its dog.  Instead of defining an option in
> parser.py and the manpage, we'd have to define them in parser.py, cases.py,
> main.py and the manpage.  K.I.S.S.!  Forget just the abstract property and you
> violate the design but nothing actually happens;

Except that your tests crash when they refer to
{self,cls}.missing_attribute.

>                                                  forget just the bit in
> ConfiguredTestMAAS and you get a traceback to remind you to define the
> abstract property;

Which happens early; no need to wait for a VM to fire up, etc.

>                    forget both and you violate the design but get a different
> traceback without the reminder.

Back to test crashes when they refer to {self,cls}.missing_attribute?

>                                  It's bad enough that manpages are generally
> like that, but in the code it's easy to avoid.

The abstract properties, once defined in TestMAAS (or a subclass), mean
that we document TestMAAS's requirements, and avert wasteful runs to
determine that we forgot to add a command-line argument to populate the
args object that we refer to in the last-but-one test case, for example.
Locality: those properties define what we need closer to where they're
needed.

>
> The option definitions on the arguments parser are effectively documentation
> in themselves, which if anything just needs to be brought to the fore.  The
> args object encapsulates them in a way that makes them easy to identify as
> arguments.  That's actually a pretty good setup — light, flexible, and clear.
> All in one place, so consistency comes naturally without enforcement.

Enforcement is useful here, and it's not clear as it is. The args object
is a pin-cushion, and does nothing more to identify arguments as such
than the abstract properties.

>                                                                       If you
> want to isolate and document the options that are relevant to test scenarios,
> I'd suggest sticking very close to the existing "args" pattern.  Once you have
> cross-module inheritance relationships separating a field's specification,
> use, and definition, you're no longer any better off than with simple cross-
> module delegation.

The abstract properties mean that we break the cross-module dependency
between maastest.cases and maastest.parser; the same names may be used
for convenience and consistency, but TestMAAS is no longer bound
directly to the command-line.

We also add a formal contract with maastest.main, in that it cannot
instantiate TestMAAS or any of its subclasses without providing values
for the missing properties.

>
> Nor do I agree with inlining prepare_parser() into module-level code.  A
> declarative list of argparse.Action as a module-level...

[lp:~allenap/maas-test/configure-abstract]

69. By Gavin Panella on 2013-11-26

Update the man page from the --help output.

[Jeroen T. Vermeulen (jtv)]

Actually I think we really just disagree about one important thing: whether forgetting to define an abstract property for one of the options will produce an appropriate error. If it does, I generally agree with your considerations. But my position is: no it doesn't, because any references the methods make to these properties are bound when they execute, not when they are defined. And by the time they execute, these properties have been set. No error, no enforcement.

But we're in luck: we can just try it! Which is what I'm doing right now. I removed the abstractproperty definitions, all of them for good measure, though I kept the base class abstract. I hope you'll agree that's a good, extreme example of forgetting to define some. Tests pass. Running maas-test seems to work just fine so far. At the time of writing it's trying to import boot images, at which point you'll agree it must have noticed if there were a problem reading the proxy, series, and architecture options if nothing else.

[lp:~allenap/maas-test/configure-abstract]

70. By Gavin Panella on 2013-11-27

Merge trunk, resolving conflicts.

71. By Gavin Panella on 2013-11-27

Update man page.

[Gavin Panella (allenap)]

> Actually I think we really just disagree about one important thing: whether
> forgetting to define an abstract property for one of the options will produce
> an appropriate error. If it does, I generally agree with your considerations.
> But my position is: no it doesn't, because any references the methods make to
> these properties are bound when they execute, not when they are defined. And
> by the time they execute, these properties have been set. No error, no
> enforcement.
>
> But we're in luck: we can just try it! Which is what I'm doing right now. I
> removed the abstractproperty definitions, all of them for good measure, though
> I kept the base class abstract. I hope you'll agree that's a good, extreme
> example of forgetting to define some. Tests pass. Running maas-test seems to
> work just fine so far. At the time of writing it's trying to import boot
> images, at which point you'll agree it must have noticed if there were a
> problem reading the proxy, series, and architecture options if nothing else.

I didn't argue that forgetting to define an abstract property would
cause an error. I argued that forgetting to set that property in a
subclass would cause an error, which it does. Removing the abstract
properties from TestMAAS doesn't demonstrate anything particularly
useful.

The purpose of the abstract properties is to define TestMAAS's
requirements close to their use. Try removing one of the command-line
options, or forgetting to set one of the properties on
ConfiguredTestMAAS, then you'll get an early warning. The args pattern
displays neither of those properties; you'll only ever get "late"
breakage.

[lp:~allenap/maas-test/configure-abstract]

72. By Gavin Panella on 2013-11-27

Switch over to disable_cache.

73. By Gavin Panella on 2013-11-27

Merge trunk, resolving lots of conflicts.

74. By Gavin Panella on 2013-11-27

Fix lint.

[Jeroen T. Vermeulen (jtv)]

Now you have me utterly confused. You quoted what I said:

> Forget just the abstract property and you
> violate the design but nothing actually happens;

And your response to that was:

> Except that your tests crash when they refer to
> {self,cls}.missing_attribute.

And I'm telling you that nothing crashes, as long as you don't _also_ forget to set the property in ConfiguredTestMAAS. If you forget the latter, it crashes either way. But you can forget just the abstract property and everything still works. And that makes the enforcement rather weak!

[Gavin Panella (allenap)]

> Now you have me utterly confused. You quoted what I said:
>
> > Forget just the abstract property and you
> > violate the design but nothing actually happens;
>
> And your response to that was:
>
> > Except that your tests crash when they refer to
> > {self,cls}.missing_attribute.
>
> And I'm telling you that nothing crashes, as long as you don't _also_ forget
> to set the property in ConfiguredTestMAAS. If you forget the latter, it
> crashes either way. But you can forget just the abstract property and
> everything still works. And that makes the enforcement rather weak!

Okay, I misunderstood your point there; I had assumed you meant not
providing a setting for the abstract property in the subclass. If you
forget to define an abstract property you're hardly better off than you
are now.

However, the point of the abstract properties is that they're nearby the
code that needs them. Right now you must marry up all references to
cls.args.* or self.args.* in maastest.cases with the command-line
options defined in maastest.parser.

It is an improvement, and that's usually our measure for what may land.

[lp:~allenap/maas-test/configure-abstract]

75. By Gavin Panella on 2013-11-27

Merge trunk, resolving conflicts.

[Jeroen T. Vermeulen (jtv)]

The mere fact that we can have a misunderstanding about which of the definitions of the same option I'm talking about speaks volumes. That complexity is a steep price to pay when there is no particularly profound divide between program options and test parameters which we need to enforce or document.

And aside from that divide between options and parameters, what does it protect us from? If we stick to a single args object, there is really just one mistake you can make with a test parameter: inconsistency between its use and its definition in the args parser. Simple, lightweight, flexible, obvious.

With the "wide interface" to parameters which you're creating here, there's a lot more to go wrong. Now it's the argparser, abstract properties, initialisations, and uses that all need to be consistent. There are now several failure modes for different (combinations of) inconsistencies. How many of those are improvements? You're increasing the odds of failing early, but at the cost of increasing the odds of failing at all.

I suppose the real problem is that it can be slow and difficult to discover mistakes in accessing those options from the main code. A current-day linter won't tell us when we're going to access undefined parameters. The abstract properties won't help with that either; they only provide a redundancy check on the copying of the parameters. But this is properly a matter of test coverage. Isn't the logical solution to address that, rather than add red tape in the hope that it will trip up mistakes? For example, I see no test to cover the instantiation of ConfiguredTestMAAS that would report any missing initialisations. If we could just pass realistic args objects to unit tests, mistakes in parameter access should be easy to beat.

[Raphaël Badin (rvb)]

I had a quick look at the code and read the ragging discussion.

My two cents: I tend to think Jeroen's point is justified: adding these abstract properties simply adds another thing you have to modify each time you want to add a new command which translates into a property. The argument that adding these properties would speed up the debugging when someone makes a mistake is only half valid: if, for instance, one has two mutually exclusive properties, only proper testing can detect this kind of problem.

[Gavin Panella (allenap)]

What I see as a fairly obvious improvement has been voted down because
it's too convenient to tightly couple the command-line to TestMAAS.

One of the dependent branches of this already breaks that: it adds a
--daemon option which is nothing to do with TestMAAS; it's an option to
select a different test suite class, among other things.

Now two things would be coupled closely to the command-line, and
TestMAAS will be free to use args.daemon if it so chooses, and perhaps
someone will make it do that one day, thus starting the process of
spaghettification.

Do you not see that this is bad?

> The mere fact that we can have a misunderstanding about which of the
> definitions of the same option I'm talking about speaks volumes.

I just misunderstood what you wrote, not the concepts involved.

> That complexity is a steep price to pay when there is no particularly
> profound divide between program options and test parameters which we
> need to enforce or document.

That's short-sighted.

> And aside from that divide between options and parameters, what does
> it protect us from? If we stick to a single args object, there is
> really just one mistake you can make with a test parameter:
> inconsistency between its use and its definition in the args
> parser. Simple, lightweight, flexible, obvious.

Simple: Briefly, while there's still a 1:1 correspondence between the
command-line and TestMAAS.

Lightweight: Indeed, but I've not proposed anything heavyweight.

Flexible: It's so flexible you can point straight ahead and still shoot
your toes off.

Obvious: As in, it's obvious it can be improved?

> With the "wide interface" to parameters which you're creating here,
> there's a lot more to go wrong. Now it's the argparser, abstract
> properties, initialisations, and uses that all need to be
> consistent. There are now several failure modes for different
> (combinations of) inconsistencies. How many of those are improvements?
> You're increasing the odds of failing early, but at the cost of
> increasing the odds of failing at all.

Come on, this is specious. The abstract properties are provided by a
package in the standard library, and they're fairly simple anyway. The
failure modes are part of the design, to fail fast so that we know early
that there's a programming error.

> I suppose the real problem is that it can be slow and difficult to
> discover mistakes in accessing those options from the main code. A
> current-day linter won't tell us when we're going to access undefined
> parameters. The abstract properties won't help with that either; they
> only provide a redundancy check on the copying of the parameters.

As I've said several times, these parameters are defined close to where
they're used. Locality is generally a desirable property. It makes us
less likely to forget or misuse, and helps our future selves and others
understand our intentions.

> But this is properly a matter of test coverage. Isn't the logical
> solution to address that, rather than add red tape in the hope that it
> will trip up mistakes?

I was able to do this change in a short amount of time. Adding tests to
cover every use of the args object would have taken a lot longe...

[lp:~allenap/maas-test/configure-abstract]

76. By Gavin Panella on 2013-11-28

Merge trunk, resolving conflicts.

77. By Gavin Panella on 2013-11-28

Merge trunk.

[Jeroen T. Vermeulen (jtv)]

I'm sorry that feelings are flaring up about this... Yes, of course I do see that spaghettification from inappropriate mixing of program options and test parameters is bad. I'm sure we all do. I just don't see it as worse, in the near future for this limited short-term project, than the cost of detailed gatekeeping.

In other words, I think this is a case where short-sightedness is appropriate for the situation. As things change, separating program options from test parameters may become more useful. But you've got to give the patterns a chance to emerge before you set them in stone. An option like --daemon does not make a pattern, especially if you're worried that actually it may still prove relevant to the test cases. Do we have a good reason to believe that review will not be enough to stop the spaghettification?

We keep mis-understanding each other about this. That in itself worries me. This is of course not what I meant:

> Come on, this is specious. The abstract properties are provided by a
> package in the standard library, and they're fairly simple anyway. The
> failure modes are part of the design, to fail fast so that we know early
> that there's a programming error.

I'm not saying abstract properties in general have to be consistent with our option definitions. Only the abstract properties we actually define for this. It has nothing to do with the standard library.

I would love to have early catching of mistakes, but the only kind of mistake that I see being caught early here is inconsistency between the abstract property definitions and the initialisations in ConfiguredTestMAAS. But it's a mistake which the existing design doesn't make possible in the first place.

You argue that testing all uses of the args object would be costly. My view is that the real cost is in testing our code at all, which we should have been doing anyway. The tests can just say "make me an args object," with perhaps some customised fields — just like we do in our customary test factories. I imagine you had much the same in mind for testing with the abstract properties.

[Gavin Panella (allenap)]

> I'm sorry that feelings are flaring up about this...  Yes, of course I do see
> that spaghettification from inappropriate mixing of program options and test
> parameters is bad.  I'm sure we all do.  I just don't see it as worse, in the
> near future for this limited short-term project, than the cost of detailed
> gatekeeping.

I think that the status-quo is worse, but even if I'm wrong, we're not
speculating: this code is done, and it's simply not "detailed
gatekeeping".

>
> In other words, I think this is a case where short-sightedness is appropriate
> for the situation.  As things change, separating program options from test
> parameters may become more useful.  But you've got to give the patterns a
> chance to emerge before you set them in stone.

We're not setting anything in stone. Decoupling TestMAAS from the
command-line is the opposite.

> An option like --daemon does not make a pattern, especially if you're
> worried that actually it may still prove relevant to the test cases.
> Do we have a good reason to believe that review will not be enough to
> stop the spaghettification?

Yes, it's already started: the args object is being passed through to
ConfiguredTestMAAS.configure().

>
> We keep mis-understanding each other about this.  That in itself worries me.
> This is of course not what I meant:
>
> > Come on, this is specious. The abstract properties are provided by a
> > package in the standard library, and they're fairly simple anyway. The
> > failure modes are part of the design, to fail fast so that we know early
> > that there's a programming error.
>
> I'm not saying abstract properties in general have to be consistent with our
> option definitions.  Only the abstract properties we actually define for this.
> It has nothing to do with the standard library.
>
> I would love to have early catching of mistakes, but the only kind of mistake
> that I see being caught early here is inconsistency between the abstract
> property definitions and the initialisations in ConfiguredTestMAAS.  But it's
> a mistake which the existing design doesn't make possible in the first place.

A run will fail eventually, true, when it tries to deference a missing
argument. That's our ultimate measure of correctness in this regard.
This proposed change moves some of these possible failures close to the
start of a run. That's an improvement.

>
> You argue that testing all uses of the args object would be costly.  My view
> is that the real cost is in testing our code at all, which we should have been
> doing anyway.  The tests can just say "make me an args object," with perhaps
> some customised fields — just like we do in our customary test factories.  I
> imagine you had much the same in mind for testing with the abstract
> properties.

The tests could also say "make me a testcase", and the abstract
properties would be there to ensure that nothing had been omitted.
That's a small improvement.

Here is the essence of the code that we've been arguing about for a
week:

{{{
=== modified file 'maastest/cases.py'
--- maastest/cases.py   2013-11-28 17:09:31 +0000
+++ maastest/cases.py   2013-12-02 09:44:27 +0000
@@ -42,1 +45,1 @@

class TestMAAS(testto...

[lp:~allenap/maas-test/configure-abstract]

78. By Gavin Panella on 2013-12-02

Merge trunk, resolving conflicts.

79. By Gavin Panella on 2013-12-05

Merge trunk, resolving one conflict.

[Raphaël Badin (rvb)]

I have the feeling we have reached a stalemate here… I know it's not a great feeling when one has invested time in a branch to see it rejected but, well, it happens. I'm afraid the only solution here is to agree to disagree on this, mark this branch rejected, and move on. Gavin?

[Graham Binns (gmb)]

I'm going to mark this branch as rejected just to clear it out of the way. Sorry Gavin; it's great work but we've pretty much reached an impasse (and besides, this branch will be bit-rotten by now).

Unmerged revisions

79. By Gavin Panella on 2013-12-05

Merge trunk, resolving one conflict.

78. By Gavin Panella on 2013-12-02

Merge trunk, resolving conflicts.

77. By Gavin Panella on 2013-11-28

Merge trunk.

76. By Gavin Panella on 2013-11-28

Merge trunk, resolving conflicts.

75. By Gavin Panella on 2013-11-27

Merge trunk, resolving conflicts.

74. By Gavin Panella on 2013-11-27

Fix lint.

73. By Gavin Panella on 2013-11-27

Merge trunk, resolving lots of conflicts.

72. By Gavin Panella on 2013-11-27

Switch over to disable_cache.

71. By Gavin Panella on 2013-11-27

Update man page.

70. By Gavin Panella on 2013-11-27

Merge trunk, resolving conflicts.

=== modified file 'docs/man/maas-test.8.rst'
--- docs/man/maas-test.8.rst	2013-11-28 11:06:23 +0000
+++ docs/man/maas-test.8.rst	2013-12-05 16:31:01 +0000
@@ -50,7 +50,7 @@
 a route to the internet.
 
 For the node, that means that both the node's own network interface card (NIC)
-and its baseboard management controller (BMC) should be on the testing network. 
+and its baseboard management controller (BMC) should be on the testing network.
 The testing system only needs one NIC on the testing network.
 
 In addition to being on the testing network, the testing system must also have
@@ -75,7 +75,8 @@
 testing network.  You will be passing this interface to `maas-test`.
 
 5. Depending on caching, the test may download and store large amounts of data
-on the testing system.  Make sure you have sufficient disk space and network bandwidth.
+on the testing system.  Make sure you have sufficient disk space and network
+bandwidth.
 
 The data that needs downloading and/or caching consists mostly of system images
 for the virtual machine, and for booting the node.  As a rule of thumb, count
@@ -100,9 +101,42 @@
 Options
 =======
 
+Positional arguments:
+---------------------
+
+interface
+  The interface on which MAAS will provide DHCP. This is the machine's
+  interface on which the node under test should be plugged-in.
+
+Optional arguments:
+-------------------
+
 -h, --help
   Show help and exit.
 
+--interactive
+  Interactive mode.  Instead of powering up the node automatically through
+  IPMI, prompt the user to turn it on manually.  In this mode there is no need
+  to specify BMC details; the MAAS enlistment process will discover them
+  automatically.
+
+--archive=archive
+  Optional package repository name.  If given, the virtual machine may install
+  packages from this additional archive as well as from the main Ubuntu
+  archive.  The archive is added to the virtual machine using
+  'add-apt-repository'; it may be either a line in the format of apt's
+  sources.list, or a personal package archive identifier in the form
+  'ppa:<user>/<ppa-name>', or a distribution component that should be enabled.
+  This is typically used to test recent versions of MAAS that are only
+  available in a PPA such as "ppa:maas-maintainers/dailybuilds".
+
+
+BMC details:
+------------
+
+BMC/IPMI details for performing the first power on of the node under test.
+Required when --interactive has not been specified.
+
 --bmc-mac=MAC
   MAC address for the node's baseboard management controller.  MAAS will
   control the node's power and boot sequence through this controller.
@@ -127,21 +161,11 @@
   Username for IPMI authentication.  Use with **--bmc-password**.
   Not needed in interactive mode.
 
---interactive
-  Interactive mode.  Instead of powering up the node automatically through
-  IPMI, prompt the user to turn it on manually.  In this mode there is no need
-  to specify BMC details; the MAAS enlistment process will discover them
-  automatically.
-
---archive=archive
-  Optional package repository name.  If given, the virtual machine may install
-  packages from this additional archive as well as from the main Ubuntu
-  archive.  The archive is added to the virtual machine using
-  'add-apt-repository'; it may be either a line in the format of apt's
-  sources.list, or a personal package archive identifier in the form
-  'ppa:<user>/<ppa-name>', or a distribution component that should be enabled.
-  This is typically used to test recent versions of MAAS that are only
-  available in a PPA such as "ppa:maas-maintainers/dailybuilds".
+
+MAAS images:
+------------
+
+Series and architecture choices pertaining to the node-under-test.
 
 --series=codename
   Code name for the Ubuntu release series that should be run on the node during
@@ -154,6 +178,12 @@
   which defaults to `generic`, so e.g. `i386/generic` may be abbreviated to
   `i386`.  The default architecture is `amd64`.
 
+
+HTTP caching proxies:
+---------------------
+
+Save bandwidth and time, especially when repeatedly invoking maas-test.
+
 --http-proxy=URL
   Use the given HTTP proxy for all downloads, both on the testing system and on
   the nodes: KVM images, MAAS boot images, and Ubuntu packages.  Like
 
=== modified file 'maastest/cases.py'
--- maastest/cases.py	2013-11-28 17:09:31 +0000
+++ maastest/cases.py	2013-12-05 16:31:01 +0000
@@ -14,6 +14,10 @@
     "TestOneNode",
     ]
 
+from abc import (
+    ABCMeta,
+    abstractproperty,
+    )
 import httplib
 import json
 import logging
@@ -25,7 +29,6 @@
 from maastest.maas_enums import NODE_STATUS
 from maastest.maasfixture import MAASFixture
 from maastest.proxyfixture import LocalProxyFixture
-import testresources
 import testtools
 from testtools.testcase import gather_details
 
@@ -49,12 +52,32 @@
             raise
 
 
-class TestMAAS(testtools.TestCase, testresources.ResourcedTestCase):
+class TestMAAS(testtools.TestCase):
     """Base class for testing machine compatibility with MAAS."""
 
-    @classmethod
-    def configure(cls, args):
-        cls.args = args
+    __metaclass__ = ABCMeta
+
+    interface = abstractproperty(doc=(
+        "The interface on which MAAS will provide DHCP."))
+    interactive = abstractproperty(doc=(
+        "Run interactively, prompting the user for input as necessary."))
+
+    bmc_mac = abstractproperty(doc=(
+        "The MAC address of the node's BMC."))
+    bmc_username = abstractproperty(doc=(
+        "The username to use when authenticating with the node's BMC."))
+    bmc_password = abstractproperty(doc=(
+        "The password to use when authenticating with the node's BMC."))
+
+    series = abstractproperty(doc=(
+        "Ubuntu series to use for enlistment and commissioning."))
+    architecture = abstractproperty(doc=(
+        "Architecture of the node being tested."))
+
+    http_proxy = abstractproperty(doc=(
+        "Optionally, an HTTP proxy to use, defined as a URL."))
+    disable_cache = abstractproperty(doc=(
+        "Don't attempt to cache downloaded resources."))
 
     fixtures = ClassFixture()
 
@@ -64,12 +87,12 @@
         # TODO: series and architecture should be script parameters.
         architecture = utils.determine_vm_architecture()
 
-        if cls.args.http_proxy is not None:
+        if cls.http_proxy is not None:
             # The user has passed an external proxy; don't start polipo,
             # just use that proxy for everything.
-            proxy_url = cls.args.http_proxy
+            proxy_url = cls.http_proxy
             logging.info("Using external proxy %s." % proxy_url)
-        elif cls.args.disable_cache:
+        elif cls.disable_cache:
             # The user has passed --disable-cache, so don't start polipo
             # and don't try to use an external proxy.
             proxy_url = ''
@@ -83,13 +106,13 @@
 
         cls.machine = KVMFixture(
             series='saucy', architecture=architecture,
-            proxy_url=proxy_url, direct_interface=cls.args.interface)
+            proxy_url=proxy_url, direct_interface=cls.interface)
         cls.fixtures.addCleanup(delattr, cls, "machine")
         cls.fixtures.useFixture(cls.machine)
 
         cls.maas = MAASFixture(
-            cls.machine, proxy_url=proxy_url, series=cls.args.series,
-            architecture=cls.args.architecture)
+            cls.machine, proxy_url=proxy_url, series=cls.series,
+            architecture=cls.architecture)
         cls.fixtures.addCleanup(delattr, cls, "maas")
         cls.fixtures.useFixture(cls.maas)
 
@@ -163,7 +186,7 @@
         """
         # TODO: only print this in debug/verbose mode.
         self.print_connection_details()
-        if self.args.interactive:
+        if self.interactive:
             self.power_up_node_interactive()
         else:
             self.power_up_node_noninteractive()
@@ -182,13 +205,13 @@
         power_address = self.find_bmc_ip_address()
         self.maas.kvm_fixture.run_command([
             'ipmipower', '-h', power_address,
-            '-u', self.args.bmc_username,
-            '-p', self.args.bmc_password, '--off'],
+            '-u', self.bmc_username,
+            '-p', self.bmc_password, '--off'],
             check_call=True)
         self.maas.kvm_fixture.run_command([
             'ipmipower', '-h', power_address,
-            '-u', self.args.bmc_username,
-            '-p', self.args.bmc_password, '--on'],
+            '-u', self.bmc_username,
+            '-p', self.bmc_password, '--on'],
             check_call=True)
 
     def find_bmc_ip_address(self):
 
=== modified file 'maastest/main.py'
--- maastest/main.py	2013-12-05 14:02:09 +0000
+++ maastest/main.py	2013-12-05 16:31:01 +0000
@@ -21,8 +21,8 @@
 
 
 def main(argv=None):
-    from maastest.parser import prepare_parser
-    args = prepare_parser().parse_args()
+    from maastest.parser import parser
+    args = parser.parse_args(argv)
 
     import logging
     logging.basicConfig(
@@ -41,18 +41,28 @@
             "poorer performance than on physical hardware.")
 
     from maastest.cases import TestOneNode
+    from maastest.console import run_console
 
     class ConfiguredTestMAAS(TestOneNode):
-        """A configured version of TestInteractiveOneNode.
+        """A configured test case for testing a node's MAAS compatibility.
 
-        ConfiguredTestMAAS is a TestMAAS use to configure it by calling
-        cls.configure.  We need to configure the class itself and not
-        the instance because the KVMFixture TestMAAS instanciates and needs
-        to configure is created at the class level.
+        `TestMAAS` has a number of abstract properties; it cannot be
+        instantiated until those properties are defined.
         """
-    ConfiguredTestMAAS.configure(args)
-
-    from maastest.console import run_console
+
+        interface = args.interface
+        interactive = args.interactive
+
+        bmc_mac = args.bmc_mac
+        bmc_username = args.bmc_username
+        bmc_password = args.bmc_password
+
+        series = args.series
+        architecture = args.architecture
+
+        http_proxy = args.http_proxy
+        disable_cache = args.disable_cache
+
     # This try/except is a band-aid to make sure the fixtures gets cleaned
     # up when a KeyboardInterrupt exception is raised.
     # This does miss out on test-level and module-level tearDowns.
 
=== modified file 'maastest/parser.py'
--- maastest/parser.py	2013-11-29 13:22:37 +0000
+++ maastest/parser.py	2013-12-05 16:31:01 +0000
@@ -11,7 +11,7 @@
 
 __metaclass__ = type
 __all__ = [
-    "prepare_parser",
+    "parser",
     ]
 
 import argparse
@@ -23,8 +23,8 @@
 
 class MAASTestArgumentParser(argparse.ArgumentParser):
 
-    def parse_args(self):
-        args = super(MAASTestArgumentParser, self).parse_args()
+    def parse_args(self, argv=None):
+        args = super(MAASTestArgumentParser, self).parse_args(argv)
         bmc_details = (args.bmc_mac, args.bmc_username, args.bmc_password)
         if args.interactive:
             if bmc_details != (None, None, None):
@@ -47,74 +47,75 @@
         return args
 
 
-latest_LTS_series = distro_info.UbuntuDistroInfo().lts()
-
-
-def prepare_parser():
-    # TODO: This is not tested sinc how maas-test will interface with
-    # checkbox is still in flux.  Right now, this is just to get the
-    # testing going.
-    parser = MAASTestArgumentParser(
-        description="Test whether or not a node is compatible with MAAS.")
-
-    parser.add_argument(
-        'interface', type=text_type,
-        help="Network interface that connects the testing system to the "
-             "dedicated testing network.  MAAS will serve DHCP on this "
-             "interface.")
-    parser.add_argument(
-        '--archive', type=text_type, default=None,
-        help="Additional personal package archive, 'sources.list' entry, or "
-             "distribution component to install on the virtual MAAS system.")
-    parser.add_argument(
-        '--interactive', action='store_true',
-        help="Interactive mode.  Prompt the user to power up the node "
-             "manually instead of doing it automatically through the BMC.")
-
-    # BMC details.
-    parser.add_argument(
-        '--bmc-mac', type=text_type,
-        help="MAC address of the node's baseboard management controller.  "
-             "Use this if the BMC is connected to the interface given as "
-             "argument. In non-interactive mode, either --bmc-mac or "
-             "--bmc-ip.  Not needed in interactive mode.")
-    parser.add_argument(
-        '--bmc-ip', type=text_type,
-        help="IP address of the node's baseboard management controller.  "
-             "Use this if the BMC is not connected to the inteface given as "
-             "argument.  Note that the IP address must not change for the "
-             "duration of the testing.  In non-interactive mode, either "
-             "--bmc-mac or --bmc-ip.  Not needed in interactive mode.")
-    parser.add_argument(
-        '--bmc-username', type=text_type,
-        help="Username for authenticating to the node's BMC.  "
-             "Not needed in interactive mode.")
-    parser.add_argument(
-        '--bmc-password', type=text_type,
-        help="Password for authenticating to the node's BMC.  "
-             "Not needed in interactive mode.")
-
-   # MAAS images.
-    parser.add_argument(
-        '--series', type=text_type,
-        choices=utils.get_supported_series(),
-        default=latest_LTS_series,
-        help="Code name for the Ubuntu release series that the node should "
-             "run.  Defaults to the latest LTS (%(default)s).")
-    parser.add_argument(
-        '--architecture', type=text_type,
-        default='amd64',
-        help="Node's CPU architecture, e.g. armhf/highbank or amd64.  "
-             "Defaults to %(default)s.")
-
-    # Proxy.
-    parser.add_argument(
-        '--http-proxy', type=text_type,
-        help="HTTP proxy to use for all downloads.  Imples --disable-cache.")
-    parser.add_argument(
-        '--disable-cache', action='store_true',
-        help="By default, maas-test uses polipo to cache all KVM images, "
-        "MAAS images and apt packages. If --disable-cache is specified, "
-        "polipo will be disabled.")
-
-    return parser
+# TODO: This is not tested since how maas-test will interface with
+# checkbox is still in flux.  Right now, this is just to get the
+# testing going.
+parser = MAASTestArgumentParser(
+    description="Test whether or not a node is compatible with MAAS.")
+
+parser.add_argument(
+    'interface', type=text_type,
+    help="Network interface that connects the testing system to the "
+         "dedicated testing network.  MAAS will serve DHCP on this "
+         "interface.")
+# TODO: Actually use the archive value.
+parser.add_argument(
+    '--archive', type=text_type, default=None,
+    help="Additional personal package archive, 'sources.list' entry, or "
+         "distribution component to install on the virtual MAAS system.")
+parser.add_argument(
+    '--interactive', action='store_true',
+    help="Interactive mode.  Prompt the user to power up the node "
+         "manually instead of doing it automatically through the BMC.")
+
+bmc_details = parser.add_argument_group(
+    "bmc details", "BMC/IPMI details for performing the first power on of "
+    "the node under test. Required when --interactive has not been "
+    "specified.")
+bmc_details.add_argument(
+    '--bmc-mac', type=text_type, metavar="MAC",
+    help="MAC address of the node's baseboard management controller.  "
+         "Use this if the BMC is connected to the interface given as "
+         "argument. In non-interactive mode, either --bmc-mac or "
+         "--bmc-ip.  Not needed in interactive mode.")
+bmc_details.add_argument(
+    '--bmc-ip', type=text_type, metavar="IP",
+    help="IP address of the node's baseboard management controller.  "
+         "Use this if the BMC is not connected to the inteface given as "
+         "argument.  Note that the IP address must not change for the "
+         "duration of the testing.  In non-interactive mode, either "
+         "--bmc-mac or --bmc-ip.  Not needed in interactive mode.")
+bmc_details.add_argument(
+    '--bmc-username', type=text_type, metavar="USERNAME",
+    help="Username for authenticating to the node's BMC.  "
+         "Not needed in interactive mode.")
+bmc_details.add_argument(
+    '--bmc-password', type=text_type, metavar="PASSWORD",
+    help="Password for authenticating to the node's BMC.  "
+         "Not needed in interactive mode.")
+
+maas_images = parser.add_argument_group(
+    "maas images", "Series and architecture choices pertaining to the "
+    "node-under-test.")
+maas_images.add_argument(
+    '--series', type=text_type,
+    choices=utils.get_supported_series(),
+    default=distro_info.UbuntuDistroInfo().lts(),
+    help="Code name for the Ubuntu release series that the node should "
+         "run.  Defaults to the latest LTS (%(default)s).")
+maas_images.add_argument(
+    '--architecture', type=text_type, default='amd64',
+    help="Node's CPU architecture, e.g. arm/highbank or amd64.  "
+         "Defaults to %(default)s.")
+
+proxy_options = parser.add_argument_group(
+    "http proxies & caching", "Save bandwidth and time, especially when "
+    "repeatedly invoking %(prog)s.")
+proxy_options.add_argument(
+    '--http-proxy', type=text_type, metavar="URL",
+    help="HTTP proxy to use for all downloads.  Imples --disable-cache.")
+proxy_options.add_argument(
+    '--disable-cache', action='store_true',
+    help="By default, maas-test uses polipo to cache all KVM images, "
+    "MAAS images and apt packages. If --disable-cache is specified, "
+    "polipo will be disabled.")
 
=== modified file 'maastest/utils.py'
--- maastest/utils.py	2013-11-29 12:38:02 +0000
+++ maastest/utils.py	2013-12-05 16:31:01 +0000
@@ -253,4 +253,7 @@
 def get_user_input(message):
     """A wrapper around raw_input() that allows us to sanely mock it.
     """
+    # TODO: Find out why it's not possible to sanely mock raw_input()
+    # without this.
+    # TODO: Don't use this unless --interactive has been set.
     return input(message)
 
=== modified file 'man/maas-test.8'
--- man/maas-test.8	2013-11-29 05:30:32 +0000
+++ man/maas-test.8	2013-12-05 16:31:01 +0000
@@ -91,7 +91,8 @@
 testing network.  You will be passing this interface to \fImaas\-test\fP\&.
 .sp
 5. Depending on caching, the test may download and store large amounts of data
-on the testing system.  Make sure you have sufficient disk space and network bandwidth.
+on the testing system.  Make sure you have sufficient disk space and network
+bandwidth.
 .sp
 The data that needs downloading and/or caching consists mostly of system images
 for the virtual machine, and for booting the node.  As a rule of thumb, count
@@ -112,11 +113,41 @@
 to power up the node.
 .UNINDENT
 .SH OPTIONS
+.SS Positional arguments:
+.INDENT 0.0
+.TP
+.B interface
+The interface on which MAAS will provide DHCP. This is the machine\(aqs
+interface on which the node under test should be plugged\-in.
+.UNINDENT
+.SS Optional arguments:
 .INDENT 0.0
 .TP
 .B \-h\fP,\fB  \-\-help
 Show help and exit.
 .TP
+.B \-\-interactive
+Interactive mode.  Instead of powering up the node automatically through
+IPMI, prompt the user to turn it on manually.  In this mode there is no need
+to specify BMC details; the MAAS enlistment process will discover them
+automatically.
+.TP
+.BI \-\-archive\fB= archive
+Optional package repository name.  If given, the virtual machine may install
+packages from this additional archive as well as from the main Ubuntu
+archive.  The archive is added to the virtual machine using
+\(aqadd\-apt\-repository\(aq; it may be either a line in the format of apt\(aqs
+sources.list, or a personal package archive identifier in the form
+\(aqppa:<user>/<ppa\-name>\(aq, or a distribution component that should be enabled.
+This is typically used to test recent versions of MAAS that are only
+available in a PPA such as "ppa:maas\-maintainers/dailybuilds".
+.UNINDENT
+.SS BMC details:
+.sp
+BMC/IPMI details for performing the first power on of the node under test.
+Required when \-\-interactive has not been specified.
+.INDENT 0.0
+.TP
 .BI \-\-bmc\-mac\fB= MAC
 MAC address for the node\(aqs baseboard management controller.  MAAS will
 control the node\(aqs power and boot sequence through this controller.
@@ -140,22 +171,11 @@
 .BI \-\-bmc\-user\fB= user
 Username for IPMI authentication.  Use with \fB\-\-bmc\-password\fP\&.
 Not needed in interactive mode.
-.TP
-.B \-\-interactive
-Interactive mode.  Instead of powering up the node automatically through
-IPMI, prompt the user to turn it on manually.  In this mode there is no need
-to specify BMC details; the MAAS enlistment process will discover them
-automatically.
-.TP
-.BI \-\-archive\fB= archive
-Optional package repository name.  If given, the virtual machine may install
-packages from this additional archive as well as from the main Ubuntu
-archive.  The archive is added to the virtual machine using
-\(aqadd\-apt\-repository\(aq; it may be either a line in the format of apt\(aqs
-sources.list, or a personal package archive identifier in the form
-\(aqppa:<user>/<ppa\-name>\(aq, or a distribution component that should be enabled.
-This is typically used to test recent versions of MAAS that are only
-available in a PPA such as "ppa:maas\-maintainers/dailybuilds".
+.UNINDENT
+.SS MAAS images:
+.sp
+Series and architecture choices pertaining to the node\-under\-test.
+.INDENT 0.0
 .TP
 .BI \-\-series\fB= codename
 Code name for the Ubuntu release series that should be run on the node during
@@ -167,6 +187,11 @@
 architecture only.  The architecture may include a sub\-architecture name,
 which defaults to \fIgeneric\fP, so e.g. \fIi386/generic\fP may be abbreviated to
 \fIi386\fP\&.  The default architecture is \fIamd64\fP\&.
+.UNINDENT
+.SS HTTP caching proxies:
+.sp
+Save bandwidth and time, especially when repeatedly invoking maas\-test.
+.INDENT 0.0
 .TP
 .BI \-\-http\-proxy\fB= URL
 Use the given HTTP proxy for all downloads, both on the testing system and on
 
=== modified file 'packages.txt'
--- packages.txt	2013-11-28 07:46:00 +0000
+++ packages.txt	2013-12-05 16:31:01 +0000
@@ -8,7 +8,6 @@
 python-netaddr
 python-netifaces
 python-six
-python-testresources
 python-testtools
 python-virtualenv
 python-xdg
@@ -17,7 +16,6 @@
 python3-mock
 python3-netaddr
 python3-six
-python3-testresources
 python3-testtools
 python3-xdg
 qemu-kvm