Skip to content
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 MDLV2000WriterTest #359

Closed
wants to merge 1 commit into from

Conversation

k-ujihara
Copy link
Contributor

This test is failed on Windows because a new line chars are '\n' in this test but MDLV2000Writer creates '\r\n' eol on Windows.

@johnmay
Copy link
Member

johnmay commented Aug 23, 2017

Ideally the writer should be generating consistent line endings '\n' independant of the system. Unfortunately the default java writers don't have this option but we can add that in.

@k-ujihara
Copy link
Contributor Author

OK. We may modify writer.newline() to write('\n') and fix tests in MDLV2000WriterTest using toString().split(System.getProperty("line.separator") .

@johnmay
Copy link
Member

johnmay commented Aug 23, 2017

Sounds good, are you okay to do that?

@k-ujihara
Copy link
Contributor Author

Yes. I'll commit it later.

@johnmay
Copy link
Member

johnmay commented Aug 23, 2017

Thanks

@k-ujihara
Copy link
Contributor Author

I made a new pull request about this issue at #360. Please check it out.

@johnmay
Copy link
Member

johnmay commented Aug 25, 2017

Presume these now pass on windows? I'm getting a failure in cdk-builder3d but think it's easy to resolve.

@johnmay
Copy link
Member

johnmay commented Aug 25, 2017

Never mind... that builder3d failure was me being stricter on mass deltas, will fix

@johnmay
Copy link
Member

johnmay commented Sep 8, 2017

resolved by #360

@johnmay johnmay closed this Sep 8, 2017
@k-ujihara k-ujihara deleted the FixMDLV2000WriterTest branch December 6, 2017 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants