Francisco Martín Brugué <franci...@email.de> added the comment: 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 ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue14183> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com