-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix XMLTest.testIndentComplicatedJsonObjectWithArrayAndWithConfig() for Windows - in the test #778
Conversation
@Madjosz Thanks for catching this! I had gotten out of the habit of testing on Windows. |
Never mind, it's working now, but I had to run: |
@stleary THE PR doesn't say there is a conflict. Are you sure you are on master when trying the pull? |
What problem does this code solve? Risks Changes to the API? Will this require a new release? Should the documentation be updated? Does it break the unit tests? Was any code refactored in this commit? Review status Because of Hacktoberfest, both #782 and #778 will be accepted and merged, in that order. Starting 3-day comment window |
I rebased both branches ontop of the master for you. |
@Madjosz Thanks, checkout and pull is working now. |
#593 and 85495fa included some workaround for Windows machines regarding linefeeds. The test
org.json.junit.XMLTest.testIndentComplicatedJsonObjectWithArrayAndWithConfig()
is currently failing on my Windows 10 machine and also I see no reason why this workaround was needed in the first place. The fileIssue593.xml
contains only\n
line feeds and no conversion is done while reading this file, thusexpected
only contains\n
. Conversely the tested methodXML.toString()
only appends hardcoded\n
.The only setting where the conversion is needed is when someone has their
.gitattributes
/Git settings or IDE configured to autoreplace linefeeds by the system default. In that case I would say it is actually correct that the test fails because it indicates a wrong setup.