Packaging’s test support module should have tests.

Using the “easy” keyword because I’m willing to review and commit partial patches by new contributors instead of waiting for one complete patch.

I've started with test for “fake_dec” and “TempdirManager”. Please let me know if that in the line you want.

Thanks in advance

Francis

I made a review on Rietveld; a mail should have been sent.

Hi Éric,
sorry but I've not received that email. Could you simply paste/send me the link (I've not used Rietveld already and I can not to find that review).
Thanks !

Follow the link titled “review” on the right of your patch in the list of files above the messages.

I've updated the patch. From your review: the class 'Mixin' is still
there as the idea was to imitate the use of 'TempdirManager' as is
used in the rest of the tests and test what it's documentation says.

Just let me know your preferences here.

Thank in advance !

The patch is updated. Let me know if I've understood your review.
Thanks !

Yes, you did.  There are a few minor edits I’ll make to the patch, but otherwise this is good to commit.

New changeset 0aad55c8ff26 by Éric Araujo in branch 'default':
Add tests for packaging.tests.support (#12659).
http://hg.python.org/cpython/rev/0aad55c8ff26

This first patch is committed!  If you have any question about the changes I made to your patch, feel free to ask them.  The most important change I did was running setUp and tearDown before and after each test method, as real unittest would do.

If you want to you write more tests, know that there are some things I want to remove or merge.  TestDistribution and create_distribution will probably be removed when I manage to work on my config module rewrite.

On the contrary, LoggingCatcher and EnvironRestorer won’t go away, so someone interested can work on testing them (LoggingCatcher: medium to hard, EnvironRestorer: piece of cake).

Finally, I’m not sure it’s useful to test two specific functions.  copy_xxmodule_c is tested by running the tests in different ways (via python -m test, python Lib/packaging/tests/test_thing.py, ../../../python test_thing.py); that’s how I made sure it worked in different situations, and tests wouldn’t have helped.  For fixup_build_ext, we know it works because we have buildbots compiling Python with various options (shared on Unix, debug on Windows) and our tests pass, so fixup_build_ext is doing its job.

Hi Éric,
Nice! and thanks for the info: I was just waiting to the check-in to 
compare and try further with the tests. Now I can just commit another 
patch by synching against the actual tip.

Cheers,

francis

I ported the patch to distutils2, but it fails because of some unittest internal problem.  Help welcome.

Hi Éric,
I've mutated test_support.py a bit and now passes in https://bitbucket.org/tarek/distutils2 changeset:6c3d67ed3adb.

I've added runTest to Tester, used os.unlink also removed the test fake_dec test as in support.py from distutils2 version is not present.

Best regards,

Francis

New changeset 659bf2a679d2 by Éric Araujo in branch 'default':
Add tests for tests.support (#12659), thanks to Francisco Martín Brugué
http://hg.python.org/distutils2/rev/659bf2a679d2

> I've added runTest to Tester
It turns out this was the only change needed.  Tarek’s repo on bitbucket is not up-to-date, I should have given you the hg.python.org link.  (You can edit .hg/hgrc in your repo and run “hg pull -u” to get missing changesets.)

When you add more tests in packaging’s test_support, please add the runTest method too, so that both versions are synchronized.  Thanks!

I've just updated “test_support.py” and tested against 'default'
and 'distutils2' (after changing the imports).

Just let me know what has to be changed ...


BTW: in distutils2 I get (not because of this change):

======================================================================
FAIL: test_bad_urls (distutils2.tests.test_pypi_simple.SimpleCrawlerTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/ci/prog/cpython/distutils2/distutils2/tests/pypi_server.py", line 68, in wrapped
    func(server=server, *args, **kwargs)
  File "/home/ci/prog/cpython/distutils2/distutils2/tests/test_pypi_simple.py", line 93, in test_bad_urls
    self.assertIn(wanted, str(v))
AssertionError: 'nonnumeric port' not found in 'Download error for http://http://svn.pythonpaste.org/Paste/wphp/trunk: [Errno -2] Name or service not known'

----------------------------------------------------------------------
 
Cheers,
francis

> I've just updated “test_support.py” and tested against 'default'
Do you mean that you have added more tests to the existing file?  If so, could you provide a patch this time?

> BTW: in distutils2 I get (not because of this change):
It’s a known failure that’s caused by a bug fix in 2.7.3.  I didn’t fix it because I need to check the original bug report for the failing test.

> If so, could you provide a patch this time?
Done. The patch is against default.

what about against distutils2? Do you need also a patch for it?

> BTW: in distutils2 I get (not because of this change):
Actually I did fix that, but depending on your Python version the conditional in the test may be wrong.  What’s the Python version you used?

>> If so, could you provide a patch this time?
> Done. The patch is against default.
packaging is only in default, so yeah :)  Thanks.

> what about against distutils2? Do you need also a patch for it?
I can port from p7g to d2, no worries.

On 02/11/2012 05:59 AM, Éric Araujo wrote:

>> BTW: in distutils2 I get (not because of this change):
> Actually I did fix that, but depending on your Python version the conditional in the test may be wrong.  What’s the Python version you used?

I did:
python2.7 runtests.py

The exact version is: python2.7 -V -->
Python 2.7.2+

and the machine is a Debian: uname -a -->
Linux random 3.1.0-1-amd64 #1 SMP Tue Jan 10 05:01:58 UTC 2012 x86_64 
GNU/Linux

Thanks for your time as mentor!

Patch updated.

BTW, I didn't get any email due the review that Éric did (does one has to be registered? I've done that now).

> Patch looks good, thanks!  BTW are you running a coverage tool to see > how far along we are?

Yes, always. The classes tested in that patch are at 100%.

The overall coverage is:

Name                          Stmts   Miss  Cover   Missing
-----------------------------------------------------------
Lib/packaging/tests/support     173     34    80%   296-301, 306-310, 334-337, 341-347, 371-390, 395-396

I've updated the patch with a test case for 'create_distribution' and one minor rename.

I personally don't like too much the structure of 'LoggingCatcherTestCase' so improvements are welcome.

Cheers,

francis