classification
Title: Increase distutils.filelist / packaging.manifest test coverage
Type: behavior Stage: resolved
Components: Distutils, Distutils2 Versions: Python 3.3, Python 3.2, Python 2.7, 3rd party
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: alexis, eric.araujo, jlove, python-dev, tarek
Priority: normal Keywords: patch

Created on 2011-04-03 19:07 by jlove, last changed 2012-02-18 01:46 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
increase_distutils_filelist_test_coverage.patch jlove, 2011-04-03 19:07 review
increase_distutils_filelist_test_coverage_v2.patch jlove, 2011-04-07 17:27 -NO COVER; combine test_translate_pattern; more expressive test names review
distutils-filelist.diff eric.araujo, 2011-06-03 16:04 review
packaging-manifest.diff eric.araujo, 2011-06-03 16:05 review
packaging-manifest_v2.diff jlove, 2011-09-24 01:27 review
packaging-manifest_v3.diff jlove, 2011-09-25 20:19 review
distutils2-manifest.diff eric.araujo, 2011-10-19 19:10
Messages (23)
msg132869 - (view) Author: Justin Love (jlove) Date: 2011-04-03 19:07
Increase test coverage of distutils/filelist.py from 49% to 100%.  One line was marked as excluded because it was a "this cannot happen" error, and I agreed.
msg132877 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-03 20:06
Thanks for the patch!  Please use the Distutils and/or Distutils2 component in the future so that some people are automatically notified of the report.

I made a review (follow “review” link on the right of the diff file).
msg133238 - (view) Author: Justin Love (jlove) Date: 2011-04-07 17:27
Removed NO COVER

Combined test_translate_pattern

Added on to some test names to make them more descriptive
msg133313 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-08 15:07
FYI, I currently only have sporadic Internet access, moreover with SSH blocked, and I want to wait for the merge of distutils2 into the stdlib before I commit some fixes, so don’t worry if this sits for a while.
msg137543 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-06-03 16:04
Hi!  I cleaned up your patch and fixed a bug: you have to call sort before calling remove_duplicates (the result in your patch had two 'a' entries).

> One line was marked as excluded because it was a "this cannot happen"
> error, and I agreed.
How about we add a test to make this it cannot happen and then remove the line?

I also ported it to packaging, the replacement for distutils; I get this failure:

FAIL: test_process_template (__main__.ManifestTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Lib/packaging/tests/test_manifest.py", line 166, in test_process_template
    action)
AssertionError: PackagingTemplateError not raised by _process_template_line

I haven’t investigated if the test is wrong or the code buggy yet; do you have any insight?
msg144458 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-23 16:52
Hi Justin, is there any way I can help you move forward with this?  Please tell if you don’t have the time, I can work on completing the patch.
msg144481 - (view) Author: Justin Love (jlove) Date: 2011-09-24 01:27
Sorry, it's been sitting at the bottom of my starred list for months, the prospect of reloading context always making it a little less attractive than something else.  I turned out I had to update to even have 'packaging'

Error was caused by an actual change in behavior - I was testing the argument-check by providing an action with no argument.  The new code assumes a single 'word' is a filename, and translates it to include, eg. 'include include' which then passes the parse.

I modified the Manifest to test against the list of actions, since this makes it harder for users to make a mistake.  Of course we could just drop the test, or only check multi-argument actions.
msg144515 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-25 01:54
> Sorry, it's been sitting at the bottom of my starred list for months,
> the prospect of reloading context always making it a little less
> attractive than something else.
I understand that very well :)

> Error was caused by an actual change in behavior
Ah, good, I had missed that change.  I’ll just adapt the test to check that one-word == include.

I think I won’t add the Manifest._actions attribute as in your patch; it does not cost much to have the list once in manifest and another time in test_manifest.

The only change I’ll make (or maybe you’d like to do it?) before committing is to add a test to make sure the line marked with “this cannot happen” really cannot happen.
msg144530 - (view) Author: Justin Love (jlove) Date: 2011-09-25 20:19
Incorporated some formatting suggestions.

Removed _actions.  Just as well, as I accidentally left in the 'blarg' bogus action ;^)

Added a test for implicit-include.

Whether 'this cannot happen' can happen depends on the output of _parse_template_line.  If you wanted to remove the catch-all and still have test backup, you'd probably have to restort to some form of fuzz-testing _process_template_line.  As the code stands now it could be removed.  I left it alone as insurance against future changes.
msg145412 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-12 16:32
I made a few edits and committed to distutils and packaging.  Then I ported the packaging patch to distutils2, which supports Python 2.4-2.7, and got this failure:

FAIL: test_glob_to_re (__main__.ManifestTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "distutils2/tests/test_manifest.py", line 88, in test_glob_to_re
    self.assertEqual(_glob_to_re('foo*'), 'foo[^/]*\\Z(?ms)')
AssertionError: 'foo[^/]*$' != 'foo[^/]*\\Z(?ms)'

I haven’t dug into it yet.
msg145525 - (view) Author: Roundup Robot (python-dev) Date: 2011-10-14 14:57
New changeset 866d098367c1 by Éric Araujo in branch '3.2':
Increase test coverage for distutils.filelist (#11751).
http://hg.python.org/cpython/rev/866d098367c1

New changeset ba894d8a2a57 by Éric Araujo in branch 'default':
Merge #11751 from 3.2
http://hg.python.org/cpython/rev/ba894d8a2a57

New changeset a30d4864a8e4 by Éric Araujo in branch 'default':
Increase test coverage for packaging.manifest (#11751).
http://hg.python.org/cpython/rev/a30d4864a8e4
msg145546 - (view) Author: Roundup Robot (python-dev) Date: 2011-10-14 16:15
New changeset 368134e10d09 by Éric Araujo in branch '2.7':
Increase test coverage for distutils.filelist (#11751).
http://hg.python.org/cpython/rev/368134e10d09
msg145882 - (view) Author: Justin Love (jlove) Date: 2011-10-19 00:21
re: test_glob_to_re

Could not reproduce (OS X 10.6)

I built branch 2.7, and also tried the expressions from glob_to_re on my system 2.7 and 3.2, all returned the (?ms) version.
msg145940 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-19 19:10
Python 2.7 contains distutils, not distutils2.  You’d have to clone hg.python.org/distutils2 and apply the patch I’m attaching.  Thanks for the help!
msg146143 - (view) Author: Justin Love (jlove) Date: 2011-10-21 22:05
Still can't reproduce (though I got one failure and three other errors)

$ patch -p 1 <distutils2-manifest.diff
$ ../devinabox/cpython/python.exe runtests.py
...
test_glob_to_re (distutils2.tests.test_manifest.ManifestTestCase) ... ok
...
msg146207 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-23 00:31
> Still can't reproduce
It only shows in 2.4 and 2.5.

> (though I got one failure and three other errors)
If these are not covered in #13170, please add them to that report.
msg146208 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-10-23 00:53
I’ve found the reason: #6665.  fnmatch.translate (used by _glob_to_re was changed to support filenames with embedded newlines.  I don’t think it’s a concern for us: As the input we give to _glob_to_re comes one line at a time from a file, we just never have filenames with newlines.

We can do one of these things for d2:
- Special-case 2.4 and 2.5 in the tests.
- Tweak _glob_to_re so that it changes the generated regex to match what we get in newer versions.
- Tweak _glob_to_re to remove the inline flags, as we don’t need them.

I think the last idea would be best.
msg147595 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-14 14:24
New changeset 3dda26cfc1d7 by Éric Araujo in branch 'default':
Increase test coverage for manifest (#11751).
http://hg.python.org/distutils2/rev/3dda26cfc1d7
msg147997 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-20 15:45
Advice from Ezio Melotti:
> I would keep the flags even if you don't need them
> someone in the present or in the future might need them, and having them doesn't harm
msg148037 - (view) Author: Roundup Robot (python-dev) Date: 2011-11-21 13:21
New changeset 723517bf6708 by Éric Araujo in branch 'default':
Make test_manifest pass on 2.4 and 2.5 (fixes #11751).
http://hg.python.org/distutils2/rev/723517bf6708
msg148041 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-11-21 13:47
Did the second idea as suggested by Ezio.
msg153181 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-12 05:47
FYI the tests revealed a bug in the code on Windows: #13193
msg153623 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-18 01:46
It was actually already known: #6884 (and then found again: #14004).
History
Date User Action Args
2012-02-18 01:46:26eric.araujosetmessages: + msg153623
2012-02-12 05:47:18eric.araujosetmessages: + msg153181
2011-11-21 13:47:45eric.araujosetstatus: open -> closed
resolution: fixed
messages: + msg148041

stage: patch review -> resolved
2011-11-21 13:21:51python-devsetmessages: + msg148037
2011-11-20 15:45:17eric.araujosetmessages: + msg147997
2011-11-14 14:24:18python-devsetmessages: + msg147595
2011-10-23 00:53:40eric.araujosetmessages: + msg146208
2011-10-23 00:31:50eric.araujosetmessages: + msg146207
2011-10-21 22:05:52jlovesetmessages: + msg146143
2011-10-19 19:10:53eric.araujosetfiles: + distutils2-manifest.diff

messages: + msg145940
2011-10-19 00:21:41jlovesetmessages: + msg145882
2011-10-14 16:15:40python-devsetmessages: + msg145546
2011-10-14 14:57:12python-devsetnosy: + python-dev
messages: + msg145525
2011-10-12 16:32:06eric.araujosetmessages: + msg145412
2011-09-25 20:19:27jlovesetfiles: + packaging-manifest_v3.diff

messages: + msg144530
2011-09-25 01:54:19eric.araujosetmessages: + msg144515
2011-09-24 01:27:49jlovesetfiles: + packaging-manifest_v2.diff

messages: + msg144481
2011-09-23 16:52:54eric.araujosetmessages: + msg144458
2011-06-03 16:05:05eric.araujosetfiles: + packaging-manifest.diff
2011-06-03 16:04:54eric.araujosetfiles: + distutils-filelist.diff
status: pending -> open
versions: - Python 3.1
title: Increase distutils.filelist test coverage -> Increase distutils.filelist / packaging.manifest test coverage
messages: + msg137543
2011-05-29 17:36:05eric.araujosetstatus: open -> pending
2011-04-08 15:07:37eric.araujosetmessages: + msg133313
2011-04-07 17:27:21jlovesetfiles: + increase_distutils_filelist_test_coverage_v2.patch

messages: + msg133238
2011-04-03 20:06:26eric.araujosetassignee: tarek -> eric.araujo

components: + Distutils2
title: Increase distutils/filelist test coverage -> Increase distutils.filelist test coverage
nosy: + alexis
versions: + 3rd party, Python 3.1, Python 2.7, Python 3.2
messages: + msg132877
stage: patch review
2011-04-03 19:43:48brett.cannonsetassignee: tarek

components: + Distutils, - Tests
nosy: + eric.araujo, tarek
2011-04-03 19:20:03JoelLuellwitzsetversions: + Python 3.3
2011-04-03 19:07:37jlovecreate