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.

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).

Removed NO COVER
Combined test_translate_pattern
Added on to some test names to make them more descriptive

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.

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?

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.

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.

> 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.

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.

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.

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

New changeset 368134e10d09 by Éric Araujo in branch '2.7':
Increase test coverage for distutils.filelist (#11751).
http://hg.python.org/cpython/rev/368134e10d09

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.

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!

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
...

> 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.

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.

New changeset 3dda26cfc1d7 by Éric Araujo in branch 'default':
Increase test coverage for manifest (#11751).
http://hg.python.org/distutils2/rev/3dda26cfc1d7

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

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

Did the second idea as suggested by Ezio.

FYI the tests revealed a bug in the code on Windows: #13193

It was actually already known: #6884 (and then found again: #14004).

stage: patch review -> resolved

messages: + msg145940

messages: + msg144530

messages: + msg144481

messages: + msg133238

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

components: + Distutils, - Tests
nosy: + eric.araujo, tarek