homepage

Hi Éric,
> - # Test that the isntalled raises an exception if the project does not
> + # Test that the installed raises an exception if the project does not
> It took me many seconds to find the change :)
The editor told me :)
> 
> + def test_installation_get_infos_with_ClientWrapper(self):
> + # Test the use of get_infos default index
> Hm, I don’t like the method name, and don’t really understand from the
> comment what it is exactly that you’re testing (I’m not very familiar with
> p7g.pypi).
All the test with get_infos use the index=something. The default is for that parameter is None. If that ocurrs than ClientWrapper will be used.
You're right, is not interesting here. May be a new name could be
test_installation_get_infos_with_default_index ... or what you wish :)
> 
> - install.install_dists = lambda x, y=None: None
> + install.install_dists = lambda x, y = None: None
> PEP 8: Never put spaces in a function (or lambda) signature.
I cannot find that exactly in pep8. Are you saying is now OK or it was
OK? (if it was OK then the editor pep8 has a bug because that was
changed when saving)
> Let me remark that even if we get to 100% line coverage, or even 100%
> branch coverage, that won’t mean that we have covered everything. 
I'm aware of that but it's a good exercise to learn something about the package.
> Two personal anecdotes to illustrate my point. In one project, I had > a branch covered but actually untested, because I used a ternary
> operator (something like "flags = 0 if case_sensitive else re.I").
Pairwise Testing could help :) but of course getting all combinations tested can be impossible. 
> Another project was a Pylons web app; I had full coverage, and then I
> tried using non-ASCII and saw that my app was incomplete.
Well if "non-ASCII" was part of the specification then is a bug in the
implementation but if not then is a feature :P
> So I think that full code coverage is mainly useful in a new project,
> where you can have 100% all the time and use that metric to avoid
> adding code without sufficient tests. For a codebase like distutils2 > that’s half legacy, half new stuff, it’s harder to achieve that,
> and maybe less useful than working on other things. 
> If you grep packaging tests for XXX|TODO|FIXME, you’ll find 22 
> entries. Fixing those may not have an
> impact on coverage numbers, but will definitely improve things. 
> There are also XXX notes in the code itself,
> open bugs on this tracker, and some dozen more in my todo lists. For > example, packaging.database is supposed to
> handle zipped eggs, but does it? I really appreciate your 
> contributions, and think that you know enough of the code
> now to take on a larger bug if you want to. :)
> 
Well I wanted to try with more line coverage (packging.run) and I got to a point where I have to ask about (just adding another XXX and I prefer to ask first before opening a bug): I'm getting 0 or None as return value for help (and the behavior is different from the command
line or from subprocess and directly calling main).
-----8<-----8<-----8<-----8<-----8<-----
diff -r e67b3a9bd2dc Lib/packaging/tests/test_run.py
--- a/Lib/packaging/tests/test_run.py	Sat Mar 03 02:38:37 2012 +0100
+++ b/Lib/packaging/tests/test_run.py	Sat Mar 03 18:01:35 2012 +0100
@@ -2,6 +2,7 @@
 
 import os
 import sys
+import logging
 from io import StringIO
 
 from packaging import install
@@ -9,6 +10,7 @@
 from packaging.run import main
 
 from test.script_helper import assert_python_ok
+from test.support import captured_stdout, captured_stderr
 
 # setup script that uses __file__
 setup_using___file__ = """\
@@ -67,6 +69,16 @@
 self.assertGreater(out, b'')
 self.assertEqual(err, b'')
 
+ # from main directly
+ args = ('--help',)
+ with captured_stdout() as out, captured_stderr() as err:
+ status = main(args)
+ self.assertGreater(out.getvalue(), '')
+ self.assertEqual(err.getvalue(), '')
+ # XXX shouldn't be 0 (no error) to be consistent with the above?
+ # notice also that the all the messages are in stdout not error
+ self.assertEqual(status, None)
+
 def test_list_commands(self):
 status, out, err = assert_python_ok('-m', 'packaging.run', 'run',
 '--list-commands')
@@ -84,6 +96,23 @@
 
 # TODO test that custom commands don't break --list-commands
 
+ def test_no_commands(self):
+ self.assertEqual(main(), 1)
+
+ def test_no_existent_action(self):
+ args = ('NO_EXISTENT',)
+ self.assertEqual(main(args), 1)
+ self.assertIn('NO_EXISTENT', self.get_logs(level=logging.ERROR)[-1])
+
+ def test_command_help(self):
+ args = ('list', '--help')
+ with captured_stdout() as out, captured_stderr() as err:
+ status = main(args)
+ # XXX shouldn't be 0 (no error) to be consistent with the
+ # test_show_help above ?
+ self.assertEqual(status, None)
+ self.assertGreater(out.getvalue(), '')
+ self.assertEqual(err.getvalue(), '')
 
 def test_suite():
 return unittest.makeSuite(RunTestCase)
 
-----8<-----8<-----8<-----8<-----8<-----
Thanks in advance,
francis