Build failure in FileWrapperResourceTheoryTest.theoryAddingFileToDirectoryAddsResource on Windows

Description

Niels,

after your most recent ResourceStore changes, we see a failure in FileWrapperResourceTheoryTest.theoryAddingFileToDirectoryAddsResource on Windows.

It looks like a fully qualified filename starting with with C:\Windows is being appended to another filename. What should happen?

http://winbuild.geo-solutions.it/jenkins/job/GeoServer-Master/org.geoserver$gs-platform/2428/testReport/junit/org.geoserver.platform.resource/FileWrapperResourceTheoryTest/theoryAddingFileToDirectoryAddsResource/

Environment

Windows GeoSolutions Jenkins

Activity

Show:
Torben Barsballe
December 4, 2015, 1:26 AM

Niels - just saw your comment. I just pushed a fix very similar to your latest one, but a bit simpler (the replace '/' with File.Seperator is unnecessary). I have a Windows dev environment I can test this on, and it works fine there.

With respect to the ResourceWrapper.path() -> I think all resource paths are required to have forward slashes, hence the 'odd' behaviour on windows. Strictly speaking, ResourceWrapper.path() should probably be returning a relative path, but absolute resource paths are valid, so should be able to be handled, both by ResourceWrapper.path() and whatever methods it calls, and by the getResource method used in the test case. Overall, it is probably fine as it is now...

Niels Charlier
December 4, 2015, 9:13 AM
Edited

Yes, it works, but only because Java in windows is being lenient about using forward slashes. It is smart enough to understand what c:/some/path means, even though it is not a well-formed path in windows. I would personally not do to this and would have left it how I did it, but okay it still works this way as well.

Torben Barsballe
December 4, 2015, 5:07 PM

Given that, going back to your fix would be better. I have pushed a slightly tidier version using Paths.toFile(null, file) rather than a string replace of '/' to '\'. Also clarified the javadoc of Paths.toFile noting that it can accept a null base dir.
(Also, it might be the Windows OS being nice rather than java; forward slashes often work from the command line)

Niels Charlier
December 4, 2015, 6:16 PM

Thanks! It is better to use that method, that's what it is for

Andrea Aime
February 15, 2017, 11:49 AM

Mass closing all resolved issues not modified in the last 4 weeks

Fixed

Assignee

Niels Charlier

Reporter

Ben Caradoc-Davies

Triage

None

Fix versions

Affects versions

Components

Priority

Highest