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 newlines in JfrSchemaFactoryTest #10

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

prdoyle
Copy link
Contributor

@prdoyle prdoyle commented Dec 27, 2021

Fixes #6.

Not sure whether this is the direction you want to go.

@gunnarmorling
Copy link
Member

gunnarmorling commented Dec 27, 2021

Thanks! Always tricky to test these platform-specifics. That said, I think the best would be to add a .gitattributes file to the repo (e.g. like this one); once that is in place, line endings will be converted to that of the local platform upon check out. Could you give this a try perhaps (it has been a long time since I've done that for the last time, it might require some fiddling for existing files when adding it after the fact Update: see here for the required steps)?

@prdoyle
Copy link
Contributor Author

prdoyle commented Dec 27, 2021

Oh interesting! You mean that the source code would have CFLF pairs in it, and therefore the string compare would match? Great idea!

@gunnarmorling
Copy link
Member

Yes exactly; this should make sure that all line endings are always consistently those of the current platform, \r\n on Windows, \n on Linux, etc. It's more of an oversight that I didn't add the .gitattributes file from the beginning (also logged this for the oss-quickstart archetype which I used for setting up this project).

@prdoyle
Copy link
Contributor Author

prdoyle commented Dec 27, 2021

I now have my source file using dos newlines, but the text blocks are still using \n newlines instead, which I think is what the spec says they'll use. Even on Windows, text blocks don't seem to use \r\n.

@gunnarmorling
Copy link
Member

@prdoyle, so after thinking some more about this, I think the most idiomatic way for solving this issue would be using AssertJ's support for custom comparators:

assertThat(rs.getString(2)).usingComparator(NORMALIZING_LINEBREAKS).isEqualTo("""
   ...
   """);

NORMALIZING_LINEBREAKS would be a String comparator which does the linebreak conversion from your helper method. WDYT? If you agree, could you update the PR accordingly? We still should have a .gittattributes file to deal with line breaks in general.

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.

Different newlines on Windows
2 participants