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

issue-252: Adopt tests for Windows OS #253

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

Boiarshinov
Copy link
Contributor

Linked issue: #252

@coveralls
Copy link

coveralls commented Mar 10, 2022

Coverage Status

Coverage remained the same at 91.793% when pulling 50ff600 on Boiarshinov:issue-252_tests_fails_on_windows into 0a61c82 on xmlunit:main.

@Boiarshinov
Copy link
Contributor Author

Wow, it's seems that Java 7 not support \R in regular expressions. I will try to fix regex and update PR

@Boiarshinov Boiarshinov force-pushed the issue-252_tests_fails_on_windows branch from 9d34f1a to 50ff600 Compare March 10, 2022 07:43
@Boiarshinov
Copy link
Contributor Author

Boiarshinov commented Mar 10, 2022

I changend \R to (\r\n?|\n) for assertj module and saved \R for assertj3 module because it's always run on Java 8.
Now all works correctly both on Windows + Java 8 and Linux + Java 7+.

@bodewig
Copy link
Member

bodewig commented Mar 10, 2022

thanks a lot @Boiarshinov !

Is it necessary to replace the %n sequence in expectAssertionError? Doesn't the format string help with the platform sensitivity? I'm really just asking here as I must admit I haven't tried to build XMLUnit on Windows for quite some time now.

@Boiarshinov
Copy link
Contributor Author

Boiarshinov commented Mar 10, 2022

Is it necessary to replace the %n sequence in expectAssertionError? Doesn't the format string help with the platform sensitivity?

In this case format() can't help because javax.xml.parsers.DocumentBuilder always parse \r\n as \n.
Let's look at this test case tightly: we have a test XML

String testXml = String.format("<a>%nx <b/>%n</a>");

which one will be format in Windows to "<a>\r\nx <b/>\r\n</a>".
After that this model will be parsed by javax.xml.parsers.DocumentBuilder in org.xmlunit.util.Convert#toDocument(javax.xml.transform.Source, javax.xml.parsers.DocumentBuilderFactory) method.
DocumentBuilder always parse both \r\n and \n as \n. So we will have Document with two nodes with values \nx and \n (I checked this by debugging).
DiffBuilder will find difference between \nx in actual document and x in expected document and throw out comparison result with message "Expected text value 'x' but was '\nx '".
If we will expect formatted line "Expected text value 'x' but was '%nx '" than %n will be convert to \r\n and assertion will fail.

That's why we should use \n instead of %n in ExpectedException configuration.

@bodewig bodewig merged commit 33fd821 into xmlunit:main Mar 15, 2022
@bodewig
Copy link
Member

bodewig commented Mar 15, 2022

Ah, I see. I didn't realize the messages the tests are asserting contain actual XML snippets.

Thank you

@bodewig bodewig added this to the 2.9.1 milestone Mar 15, 2022
bodewig added a commit that referenced this pull request Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants