Clojure

pprint tests fail on Windows, expecting \n

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: Release 1.5
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    Windows 7
  • Patch:
    Code and Test

Description

New pprint tests were committed recently, but they fail on Windows because the tests check for \n, while pprint seems to output \r\n. A log with the test failures is attached.

The first failing commit is https://github.com/clojure/clojure/commit/4ca0f7ea17888ba7ed56d2fde0bc2d6397e8e1c0

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Andy Fingerhut added a comment -

Patch clj-1076-fix-tests-on-windows-patch-v1.txt dated Sep 29 2012 when applied to the particular commit that Ivan mentions causes the tests to pass when I run "ant" on a Windows 7 machine for me, and it continues to pass all tests on Mac OS X 10.6.8, too.

I may be doing something wrong, but when I try to run "ant" to build and test on Windows 7 with latest Clojure master, with or without this patch, it fails right at the beginning of the tests because it can't find clojure.test.generative. I'm probably doing something wrong somewhere. Ivan, would you be able to test this patch on Windows with the latest Clojure master to see if all tests pass for you now?

Show
Andy Fingerhut added a comment - Patch clj-1076-fix-tests-on-windows-patch-v1.txt dated Sep 29 2012 when applied to the particular commit that Ivan mentions causes the tests to pass when I run "ant" on a Windows 7 machine for me, and it continues to pass all tests on Mac OS X 10.6.8, too. I may be doing something wrong, but when I try to run "ant" to build and test on Windows 7 with latest Clojure master, with or without this patch, it fails right at the beginning of the tests because it can't find clojure.test.generative. I'm probably doing something wrong somewhere. Ivan, would you be able to test this patch on Windows with the latest Clojure master to see if all tests pass for you now?
Andy Fingerhut made changes -
Field Original Value New Value
Attachment clj-1076-fix-tests-on-windows-patch-v1.txt [ 11531 ]
Andy Fingerhut made changes -
Patch Code and Test [ 10002 ]
Hide
Permalink
Ivan Kozik added a comment - - edited

All tests pass on Windows 7 here with the patch.

Ant can't find my test.generative either because it isn't in my "${maven.test.classpath}". I put it in CLASSPATH and modified my build.xml like this:

<java classname="clojure.main" failonerror="false" fork="true">
<classpath>
+ <pathelement path="${java.class.path}"/>
<pathelement path="${maven.test.classpath}"/>

Show
Ivan Kozik added a comment - - edited All tests pass on Windows 7 here with the patch. Ant can't find my test.generative either because it isn't in my "${maven.test.classpath}". I put it in CLASSPATH and modified my build.xml like this: <java classname="clojure.main" failonerror="false" fork="true"> <classpath> + <pathelement path="${java.class.path}"/> <pathelement path="${maven.test.classpath}"/>
Hide
Permalink
Andy Fingerhut added a comment -

Just as a rough idea of how often people are hitting this issue, CLJ-1123 was opened on Dec 9 2012 and then closed when the ticket creator realized it was a duplicate of this one.

Show
Andy Fingerhut added a comment - Just as a rough idea of how often people are hitting this issue, CLJ-1123 was opened on Dec 9 2012 and then closed when the ticket creator realized it was a duplicate of this one.
Hide
Permalink
Mike Anderson added a comment -

Hi there is this likely to get fixed soon?

I'd like to help contribute some more patches to Clojure but it's tricky to do when I can't get the build to work

Show
Mike Anderson added a comment - Hi there is this likely to get fixed soon? I'd like to help contribute some more patches to Clojure but it's tricky to do when I can't get the build to work
Hide
Permalink
Andy Fingerhut added a comment -

I do not know if or when this patch will be committed to Clojure.

I can tell you that you can apply the patch to your own local copy of the Clojure source code, and then develop new Clojure patches based upon that version. The patch that fixes this problem only affects one test file, so it is unlikely to conflict with any changes you develop and submit.

Show
Andy Fingerhut added a comment - I do not know if or when this patch will be committed to Clojure. I can tell you that you can apply the patch to your own local copy of the Clojure source code, and then develop new Clojure patches based upon that version. The patch that fixes this problem only affects one test file, so it is unlikely to conflict with any changes you develop and submit.
Hide
Permalink
Mike Anderson added a comment -

I can confirm this patch works fine for me on Windows with Maven/Eclipse

Suggest this patch gets pushed through approval and applied ASAP? It's a pretty obvious fix that is breaking the build....

Show
Mike Anderson added a comment - I can confirm this patch works fine for me on Windows with Maven/Eclipse Suggest this patch gets pushed through approval and applied ASAP? It's a pretty obvious fix that is breaking the build....
Hide
Permalink
Stuart Halloway added a comment -

This patch is sloppy – it makes unnecessary whitespace changes in several places.

Would it be better to make the tests trailing whitespace agnostic? Otherwise this feels like poking and prodding until the build box is happy.

Show
Stuart Halloway added a comment - This patch is sloppy – it makes unnecessary whitespace changes in several places. Would it be better to make the tests trailing whitespace agnostic? Otherwise this feels like poking and prodding until the build box is happy.
Stuart Halloway made changes -
Approval Incomplete [ 10006 ]
Hide
Permalink
Andy Fingerhut added a comment -

Patch clj-1076-fix-tests-on-windows-patch-v2.txt dated Mar 2, 2013 fixes pprint tests on Windows in a different way: Removing all occurrences of carriage return (\r) characters in the output of pprint before comparing it to the expected string.

I tried simply doing str/trim-newline to remove newlines and carriage returns at the end of the string, but that does not make the tests pass. They still fail due to carriage returns in the middle of the string.

Show
Andy Fingerhut added a comment - Patch clj-1076-fix-tests-on-windows-patch-v2.txt dated Mar 2, 2013 fixes pprint tests on Windows in a different way: Removing all occurrences of carriage return (\r) characters in the output of pprint before comparing it to the expected string. I tried simply doing str/trim-newline to remove newlines and carriage returns at the end of the string, but that does not make the tests pass. They still fail due to carriage returns in the middle of the string.
Andy Fingerhut made changes -
Attachment clj-1076-fix-tests-on-windows-patch-v2.txt [ 11886 ]
Hide
Permalink
Andy Fingerhut added a comment -

Presumptuously changing Approval from Incomplete back to None, since there is a new patch attached that should address the reason it was marked Incomplete.

Show
Andy Fingerhut added a comment - Presumptuously changing Approval from Incomplete back to None, since there is a new patch attached that should address the reason it was marked Incomplete.
Andy Fingerhut made changes -
Approval Incomplete [ 10006 ]

People

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4#649-r158309) | Report a problem
Powered by a free Atlassian JIRA open source license for Clojure. Try JIRA - bug tracking software for your team.