Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2414#discussion_r150402419
--- Diff:
storm-core/test/jvm/org/apache/storm/daemon/supervisor/ContainerTest.java ---
@@ -125,7 +125,8 @@ public void testKill() throws Exception {
private static final Joiner PATH_JOIN =
Joiner.on(File.separator).skipNulls();
private static final String DOUBLE_SEP = File.separator +
File.separator;
static String asAbsPath(String ... parts) {
- return (File.separator +
PATH_JOIN.join(parts)).replace(DOUBLE_SEP, File.separator);
+ String path = (File.separator +
PATH_JOIN.join(parts)).replace(DOUBLE_SEP, File.separator);
--- End diff --
I think starting a path with `/` or `\` is inherently going to be fragile
on Windows. It might be better to remove the initial file separator from here,
and throw an exception if the first path part is `File.separator` . Then you
can probably also get rid of the `DOUBLE_SEP` replacement. Same check should be
in `asPath`. In order to work on Windows, paths should start from e.g.
`java.io.tmpdir` or the current working directory instead of the filesystem
root I think.
---
