Fixes assertions for some test that failed on Windows 7 and Windows 8.1 #553
Conversation
| @@ -235,7 +235,7 @@ public void should_be_able_to_read_asset() throws FileNotFoundException { | |||
| Document document = asciidoctor.load(DOCUMENT, options); | |||
| File inputFile = classpath.getResource("rendersample.asciidoc"); | |||
| String content = document.readAsset(inputFile.getAbsolutePath(), new HashMap<Object, Object>()); | |||
| assertThat(content, is(IOUtils.readFull(new FileReader(inputFile)))); | |||
| assertThat(content, is(IOUtils.readFull(new FileReader(inputFile)).replaceAll("\r","\\"))); | |||
There was a problem hiding this comment.
Actually, we really want to avoid doing this type of hack, even in tests. I looked at the implementation of IOUtils.readFull and it looks very strange to me. Java should be reading the file the same on all systems, but it's not.
There was a problem hiding this comment.
Upgraded to last version of JDK (131) and error banished 🤷♂️ I'll rollback the change.
| if (Platform.getNativePlatform().OS == Platform.OS.WINDOWS) | ||
| result.contains("""src="${imagesOutDir.absolutePath.replaceAll("\\\\", "//")}/${imageFileName}.png""") | ||
| else | ||
| result.contains("""src="${imagesOutDir.absolutePath}/${imageFileName}.png""") |
There was a problem hiding this comment.
I don't think we should be checking for absolute paths in the output. We would never want the HTML to point to an absolute path. But this is caused by the fact that we're passing an absolute path to imagesdir. I think imagesdir needs to be set to a relative path and then we don't have to worry about replacing any system path separators.
There was a problem hiding this comment.
At some point the usage of 'imagesdir 'was added. We could completely remove it or just set any value for the sake of ensuring imagesdir works as espected.
| @@ -27,8 +27,7 @@ class WhenDitaaDiagramIsRendered extends Specification { | |||
|
|
|||
| given: | |||
| String imageFileName = UUID.randomUUID() | |||
| File imagesOutDir = testFolder.newFolder() | |||
| File createdImage = new File(imagesOutDir, "${imageFileName}.png") | |||
| File imagesOutDir = new File(testFolder.root, "images-dir") | |||
There was a problem hiding this comment.
Without looking at the codenarc report I guess the build failed because this could be a string with single quotes instead of double quotes.
Also I could imagine that codenarc prefers .name over .getName() but I am not sure about that.
There was a problem hiding this comment.
I was checking right now. Really this is my worst PR in years XD but I take it with humor.
There was a problem hiding this comment.
I know that feeling, also happens to me very often at the moment :-D
There was a problem hiding this comment.
It complains about the GString only, but I will change the getName anyway
|
Appveyor also made it to our known rouge error. |
Fixes the first and third issues mentioned in #550 (comment).
Fixes only affect test assertions so they work both for *nix and Windows.