"manage" support for packages #13
Conversation
including automatic and recursive loading of submodules
jesteria
force-pushed
the
jsl/manage-package
branch
from
80dcaab
to
a98406a
Compare
|
@thcrock Whenever you get a chance to take a look, and if this looks reasonable to you, I can merge it 👌 |
| DATA_DIR = os.path.join(TEST_DIR, 'data') | ||
|
|
||
|
|
||
| def pm(): |
There was a problem hiding this comment.
Is this used? I'm not sure if this was left in from some manual testing or pytest magic I'm not aware of.
There was a problem hiding this comment.
There aren't any committed references to pm, I don't think, no; it's a simple tool to provide test debugging to what is currently just a plain unittest suite. (With other set-ups you can simply run tests with the flag --pdb; but, for the moment, you can't here, so I added this for debugging.)
| @cmd('target') | ||
| def subcommand(args): | ||
| target = getattr(test_argcmdr, args.target) | ||
| target.declare_success() |
There was a problem hiding this comment.
It wasn't totally clear on first read what these test/data files were and how they fit in. Looking at the test_argcmdr file helped, but just adding a bit of context as to how these commands should be used would be helpful when reading.
There was a problem hiding this comment.
I can understand how the diff might've been unclear, but I'm not entirely sure what you mean beyond that. These are just test fixtures, under the path test/data/. Do you mean a comment to reinforce this in each of these files?
There was a problem hiding this comment.
I think of fixtures as being data, not code. Not that there's a problem with this, but code fixtures are more opinionated about the context in which they are run; of course, data fixtures being plopped in a directory without context is its own problem, and generally leads to them being deprecated without anybody knowing.
In this case, it appears that it needs to specifically be called by a subclass of TryExecuteTestCase as the target, but the import and getattr implies a more generic usability.
|
Looks good! |
|
Thanks!! |
managecommand support for management packagesincluding automatic and recursive loading of submodules