jfr-analytics icon indicating copy to clipboard operation
jfr-analytics copied to clipboard

Fix newlines in JfrSchemaFactoryTest

Open prdoyle opened this issue 3 years ago • 5 comments

Fixes #6.

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

prdoyle avatar Dec 27 '21 17:12 prdoyle

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)?

gunnarmorling avatar Dec 27 '21 17:12 gunnarmorling

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

prdoyle avatar Dec 27 '21 18:12 prdoyle

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).

gunnarmorling avatar Dec 27 '21 18:12 gunnarmorling

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.

prdoyle avatar Dec 27 '21 18:12 prdoyle

@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.

gunnarmorling avatar Dec 28 '21 11:12 gunnarmorling