classification
Title: keyword.py main does not preserve line endings when rewriting keyword file
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, eric.araujo, gregmalcolm, r.david.murray, zach.ware
Priority: normal Keywords: patch

Created on 2013-04-24 15:29 by zach.ware, last changed 2013-04-25 16:07 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
test_keyword_cleanup.diff zach.ware, 2013-04-24 15:29 review
keyword_preserve_nl.patch r.david.murray, 2013-04-25 14:43 review
test_keyword_patch.diff zach.ware, 2013-04-25 15:15 David's patch plus a bit review
Messages (5)
msg187710 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-04-24 15:29
(Copying the nosy list from issue9607)

test_keyword has a couple of failures on Windows, all due to newline issues--see for example http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/1845/steps/test/logs/stdio.  

test_keywords_py_without_markers_produces_error looks for a line ending with '\n', but Windows ends it with '\r\n'.  

test_real_grammar_and_keyword_file, on the other hand, doesn't fail on my machine, due to the hg eol extension being enabled, but the cause of failure is filecmp.cmp working only in binary mode and paying no attention to line endings.

The attached patch fixes both failures, with and without the eol extension, by using a private _compare_files function instead of filecmp.cmp.  The private function makes use of universal newlines to avoid issue.  Also, all instances of ``self.addCleanup(lambda ...)`` have had the lambda removed as suggested by √Čric Araujo in msg187567.
msg187780 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-25 12:57
The filecmp test failure is not because of using filecmp.  We want a binary comparison: the line endings of the generated file should match the line endings of the input file.  So either the _copy_file_without_generated_keywords code is buggy, or this is a real bug in keyword.py.  Probably the former.

As for addCleanup, I prefer the lambda version.  But I'm fine with using the non-lambda version if that is more common in the test suite.
msg187787 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-25 14:43
Or both.

Zach, can you try this patch on Windows?  I tested it by setting my local keywords.py file to have DOS line endings, and it seems to work correctly.

Although this turns out to be a bug, it has never bothered anyone, so I don't have any intent to backport it.
msg187789 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-04-25 15:15
Your patch works for me.  And for what it's worth, I agree about not backporting; keywords are frozen and never going to change in 2.7 or 3.3 anyway.

I did find one other small bug in test_keyword; running the test via "-m test.test_keyword", test_real_grammar_and_keyword_file was skipped.  So I've patched your patch to include a fix; GRAMMAR_FILE is now an absolute path with relative elements derived from test_keyword.__file__ rather than a relative path.
msg187790 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-25 16:07
I've committed this (in 58b0e301b78a), but it looks like the email to the tracker hasn't come through yet.  I'm going to blithely assume this will fix the buildbot failures, and will reopen it if I'm wrong.
History
Date User Action Args
2013-04-25 16:07:39r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg187790

stage: resolved
2013-04-25 15:15:48zach.waresetfiles: + test_keyword_patch.diff

messages: + msg187789
2013-04-25 14:43:07r.david.murraysetfiles: + keyword_preserve_nl.patch

messages: + msg187787
title: Fix test_keyword on Windows, clean up addCleanup -> keyword.py main does not preserve line endings when rewriting keyword file
2013-04-25 12:57:17r.david.murraysetmessages: + msg187780
2013-04-24 15:29:10zach.warecreate