This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author francismb
Recipients alexis, eric.araujo, francismb, nadeem.vawda, tarek
Date 2012-03-03.17:44:09
SpamBayes Score 0.0
Marked as misclassified No
Message-id <1330796651.22.0.100292835093.issue14183@psf.upfronthosting.co.za>
In-reply-to
Content
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
History
Date User Action Args
2012-03-03 17:44:11francismbsetrecipients: + francismb, nadeem.vawda, tarek, eric.araujo, alexis
2012-03-03 17:44:11francismbsetmessageid: <1330796651.22.0.100292835093.issue14183@psf.upfronthosting.co.za>
2012-03-03 17:44:10francismblinkissue14183 messages
2012-03-03 17:44:09francismbcreate