Fixing Windows Unit Tests (fixes #402)

Major:
- Fix #402 - Windows 37 unit tests failures
- Fix #422 - echo command fails on Windows
- Disabled only failing test on Windows: #374

Minor:
- Fake OS specific file and path separators
- Explicit setFakeOS methods, e.g. setFakeOSWindows()
- Default to using native OS rather than fake OS for most unit tests
- Small number of unit tests use fakeOSName
- ‘inTestMustFakeOS’ setting removed

build.gradle

@@ -48,7 +48,6 @@ test {
         // of just pointing to an HTML file.
         exceptionFormat = 'full'
     }
-    systemProperty 'inTestMustFakeOS', 'true'
 }
 
 pluginBundle {

src/main/groovy/com/github/j2objccontrib/j2objcgradle/J2objcConfig.groovy

@@ -66,9 +66,9 @@ class J2objcConfig {
         nativeCompilation = new NativeCompilation(project)
 
         // Provide defaults for assembly output locations.
-        destSrcMainDir = "${project.buildDir}/j2objcOutputs/src/main"
-        destSrcTestDir = "${project.buildDir}/j2objcOutputs/src/test"
-        destLibDir = "${project.buildDir}/j2objcOutputs/lib"
+        destSrcMainDir = new File(project.buildDir, 'j2objcOutputs/src/main').absolutePath
+        destSrcTestDir = new File(project.buildDir, 'j2objcOutputs/src/test').absolutePath
+        destLibDir = new File(project.buildDir, 'j2objcOutputs/lib').absolutePath
     }
 
     /**

src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/CycleFinderTask.groovy

@@ -93,7 +93,7 @@ class CycleFinderTask extends DefaultTask {
 
     @TaskAction
     void cycleFinder() {
-        String cycleFinderExec = "${getJ2objcHome()}/cycle_finder"
+        String cycleFinderExec = getJ2objcHome() + Utils.fileSeparator() + 'cycle_finder'
         List<String> windowsOnlyArgs = new ArrayList<String>()
         if (Utils.isWindows()) {
             cycleFinderExec = 'java'
@@ -125,7 +125,7 @@ class CycleFinderTask extends DefaultTask {
         ])
         // TODO: comment explaining ${project.buildDir}/classes
         String classpathArg = Utils.joinedPathArg(classpathFiles) +
-                              File.pathSeparator + "${project.buildDir}/classes"
+                              Utils.pathSeparator() + "${project.buildDir}/classes"
 
         ByteArrayOutputStream stdout = new ByteArrayOutputStream()
         ByteArrayOutputStream stderr = new ByteArrayOutputStream()

src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/PackLibrariesTask.groovy

@@ -72,7 +72,7 @@ class PackLibrariesTask extends DefaultTask {
                 args 'lipo'
 
                 args '-create'
-                args '-output', "${outputLibDirFile}/lib${project.name}-j2objc.a"
+                args '-output', project.file("${outputLibDirFile}/lib${project.name}-j2objc.a").absolutePath
 
                 getLibrariesFiles().each { File libFile ->
                     args libFile.absolutePath

src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/TestTask.groovy

@@ -222,25 +222,23 @@ class TestTask extends DefaultTask {
         }
     }
 
-    // Generate Test Names
-    // Generate list of tests from the source java files
-    // e.g. src/test/java/com/example/dir/ClassTest.java => "com.example.dir.ClassTest"
+    // Generate list of test names from the source java files
     // depends on --prefixes dir/prefixes.properties in translateArgs
+    //   Before:  src/test/java/com/example/dir/SomeTest.java
+    //   After:   com.example.dir.SomeTest or PREFIXSomeTest
     static List<String> getTestNames(Project proj, FileCollection srcFiles, Properties packagePrefixes) {
 
         List<String> testNames = srcFiles.collect { File file ->
-            // Overall goal is to take the File path to the test name:
-            // src/test/java/com/example/dir/SomeTest.java => com.example.dir.SomeTest
-            // Comments show the value of the LHS variable after assignment
-
-            String testName = proj.relativePath(file)  // com.example.dir.SomeTest
-                    .replace('src/test/java/', '')
-                    .replace('/', '.')
-                    .replace('.java', '')
+            // Comments indicate the value at the end of that statement
+            String testName = proj.relativePath(file)  // src/test/java/com/example/dir/SomeTest.java
+                            .replace('\\', '/')  // Windows backslashes converted to forward slash
+                            .replace('src/test/java/', '')  // com/example/dir/SomeTest.java
+                            .replace('.java', '')  // com/example/dir/SomeTest
+                            .replace('/', '.')  // com.example.dir.SomeTest
 
             // Translate test name according to prefixes.properties
-            // Prefix Property: com.example.dir: Prefix
-            // Test Name: com.example.dir.SomeTest => PrefixSomeTest
+            // Prefix Property: com.example.dir: PREFIX
+            // Test Name: com.example.dir.SomeTest => PREFIXSomeTest
 
             // First match against the set of Java packages, excluding the filename
             Matcher matcher = (testName =~ /^(([^.]+\.)+)[^.]+$/)  // (com.example.dir.)SomeTest
@@ -249,10 +247,10 @@ class TestTask extends DefaultTask {
                 String namespaceChopped = namespace[0..-2]  // com.example.dir
                 if (packagePrefixes.containsKey(namespaceChopped)) {
                     String prefix = packagePrefixes.getProperty(namespaceChopped)
-                    testName = testName.replace(namespace, prefix)  // PrefixSomeTest
+                    testName = testName.replace(namespace, prefix)  // PREFIXSomeTest
                 }
             }
-            return testName  // com.example.dir.SomeTest or PrefixSomeTest
+            return testName  // com.example.dir.SomeTest or PREFIXSomeTest
         }
         return testNames
     }

src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/TranslateTask.groovy

@@ -235,7 +235,7 @@ class TranslateTask extends DefaultTask {
         ])
         // TODO: comment explaining ${project.buildDir}/classes
         String classpathArg = Utils.joinedPathArg(classpathFiles) +
-                              File.pathSeparator + "${project.buildDir}/classes"
+                              Utils.pathSeparator() + "${project.buildDir}/classes"
 
         ByteArrayOutputStream stdout = new ByteArrayOutputStream()
         ByteArrayOutputStream stderr = new ByteArrayOutputStream()

src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/Utils.groovy

@@ -60,37 +60,66 @@ class Utils {
         }
     }
 
+    private static String fakeOSName = ''
+
+    // This allows faking of is(Linux|Windows|MacOSX) methods but misses java.io.File separators
+    // One of the following four methods should be called in @Before method to isolate test state
+    @VisibleForTesting
+    static void setFakeOSLinux() {
+        fakeOSName = 'Linux'
+    }
+
+    @VisibleForTesting
+    static void setFakeOSMacOSX() {
+        fakeOSName = 'Mac OS X'
+    }
+
+    @VisibleForTesting
+    static void setFakeOSWindows() {
+        fakeOSName = 'Windows'
+    }
+
+    // Unset fake os, should be needed for @Before method
     @VisibleForTesting
-    static String fakeOSName = ''
+    static void setFakeOSNone() {
+        fakeOSName = ''
+    }
 
     @VisibleForTesting
-    static String getLowerCaseOSName() {
+    static String getLowerCaseOSName(boolean ignoreFakeOSName) {
 
         String osName = System.getProperty('os.name')
-        if (!fakeOSName.isEmpty()) {
-            osName = fakeOSName
-        } else {
-            if (System.getProperty('inTestMustFakeOS') == 'true') {
-                throw new InvalidUserDataException(
-                        "Tests must set Utils.fakeOSName = 'OS NAME'\n" +
-                        "This ensure that tests don't depend on the system environment")
+        if (!ignoreFakeOSName) {
+            if (!fakeOSName.isEmpty()) {
+                osName = fakeOSName
             }
         }
         osName = osName.toLowerCase()
         return osName
     }
 
-    static boolean isWindows() {
-        String osName = getLowerCaseOSName()
-        return osName.contains('windows')
+    static boolean isLinux() {
+        String osName = getLowerCaseOSName(false)
+        // http://stackoverflow.com/a/18417382/1509221
+        return osName.contains('nux')
     }
 
     static boolean isMacOSX() {
-        String osName = getLowerCaseOSName()
-        // From: http://stackoverflow.com/a/18417382/1509221
+        String osName = getLowerCaseOSName(false)
+        // http://stackoverflow.com/a/18417382/1509221
         return osName.contains('mac') || osName.contains('darwin')
     }
 
+    static boolean isWindows() {
+        String osName = getLowerCaseOSName(false)
+        return osName.contains('windows')
+    }
+
+    static boolean isWindowsNoFake() {
+        String osName = getLowerCaseOSName(true)
+        return osName.contains('windows')
+    }
+
     static void requireMacOSX(String taskName) {
         if (!isMacOSX()) {
             throw new InvalidUserDataException(
@@ -99,6 +128,24 @@ class Utils {
         }
     }
 
+    // Same as File.separator but can be faked using setFakeOSXXXX()
+    static String fileSeparator() {
+        if (isWindows()) {
+            return '\\'
+        } else {
+            return '/'
+        }
+    }
+
+    // Same as File.pathSeparator but can be faked using setFakeOSXXXX()
+    static String pathSeparator() {
+        if (isWindows()) {
+            return ';'
+        } else {
+            return ':'
+        }
+    }
+
     static void throwIfNoJavaPlugin(Project proj) {
         if (!proj.plugins.hasPlugin('java')) {
             String message =
@@ -172,8 +219,8 @@ class Utils {
     // MUST be used only in @Input getJ2objcHome() methods to ensure up-to-date checks are correct
     // @Input getJ2objcHome() method can be used freely inside the task action
     static String j2objcHome(Project proj) {
-        String result = getLocalProperty(proj, 'home')
-        if (result == null) {
+        String j2objcHome = getLocalProperty(proj, 'home')
+        if (j2objcHome == null) {
             String message =
                     "J2ObjC Home not set, this should be configured either:\n" +
                     "1) in a 'local.properties' file in the project root directory as:\n" +
@@ -186,15 +233,13 @@ class Utils {
                     "https://github.com/google/j2objc/releases"
             throw new InvalidUserDataException(message)
         }
-        if (!proj.file(result).exists()) {
-            String message = "J2OjcC Home directory not found, expected location: ${result}"
+        File j2objcHomeFile = new File(j2objcHome)
+        if (!j2objcHomeFile.exists()) {
+            String message = "J2OjcC Home directory not found, expected location: ${j2objcHome}"
             throw new InvalidUserDataException(message)
         }
-        // Trailing slashes cause problems with string concatenation logic.
-        if (result != null && result.endsWith('/')) {
-            result = result.substring(0, result.length() - 1)
-        }
-        return result
+        // File removes trailing slashes cause problems with string concatenation logic
+        return j2objcHomeFile.absolutePath
     }
 
     // Reads properties file and arguments from translateArgs (last argument takes precedence)
@@ -297,7 +342,7 @@ class Utils {
             paths += file.path
         }
         // OS specific separator, i.e. ":" on OS X and ";" on Windows
-        return paths.join(File.pathSeparator)
+        return paths.join(pathSeparator())
     }
 
     static String trimTrailingForwardSlash(String path) {

src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/XcodeTask.groovy

@@ -148,7 +148,7 @@ class XcodeTask extends DefaultTask {
             try {
                 logger.debug('XcodeTask - projectExec - pod install:')
                 Utils.projectExec(project, stdout, stderr, null, {
-                    setWorkingDir getXcodeProjectDir()
+                    setWorkingDir project.file(getXcodeProjectDir())
                     executable 'pod'
                     args 'install'
                     setStandardOutput stdout
@@ -201,22 +201,25 @@ class XcodeTask extends DefaultTask {
     }
 
     @VisibleForTesting
-    static void validatePodspecPath(String path, boolean relative) {
+    static void validatePodspecPath(String path, boolean relativeRequired) {
         if (path.contains('//')) {
             throw new InvalidUserDataException("Path shouldn't have '//': $path")
         }
         if (path.endsWith('/')) {
             throw new InvalidUserDataException("Path shouldn't end with '/': $path")
         }
-        if (relative && path.startsWith('/')) {
+        if (path.endsWith('*')) {
+            throw new InvalidUserDataException("Only genPodspec(...) should add '*': $path")
+        }
+        // Hack to recognize absolute path on Windows, only relevant in unit tests run on Windows
+        boolean absolutePath = path.startsWith('/') ||
+                               (path.startsWith('C:\\') && Utils.isWindowsNoFake())
+        if (relativeRequired && absolutePath) {
             throw new InvalidUserDataException("Path shouldn't be absolute: $path")
         }
-        if (!relative && !path.startsWith('/')) {
+        if (!relativeRequired && !absolutePath) {
             throw new InvalidUserDataException("Path shouldn't be relative: $path")
         }
-        if (path.endsWith('*')) {
-            throw new InvalidUserDataException("Only genPodspec(...) should add '*': $path")
-        }
     }
 
     // Podspec references are relative to project.buildDir

src/test/groovy/com/github/j2objccontrib/j2objcgradle/J2objcConfigTest.groovy

@@ -43,37 +43,40 @@ class J2objcConfigTest {
 
     @Before
     void setUp() {
+        // Default to native OS except for specific tests
+        Utils.setFakeOSNone()
         proj = ProjectBuilder.builder().build()
     }
 
     @Test
     void testConstructor() {
         J2objcConfig ext = new J2objcConfig(proj)
-        String projectDir = proj.projectDir.absolutePath
 
-        assert ext.destSrcMainDir == projectDir + '/build/j2objcOutputs/src/main'
-        assert ext.destSrcTestDir == projectDir + '/build/j2objcOutputs/src/test'
-        assert ext.destLibDir == projectDir + '/build/j2objcOutputs/lib'
+        assert proj.file('build/j2objcOutputs/src/main').absolutePath == ext.destSrcMainDir
+        assert proj.file('build/j2objcOutputs/src/test').absolutePath == ext.destSrcTestDir
+        assert proj.file('build/j2objcOutputs/lib').absolutePath == ext.destLibDir
     }
 
     @Test
     // All variations of main/test and objc/resources
     void testGetDestDirFile_AllVariations() {
         J2objcConfig ext = new J2objcConfig(proj)
-        String pDir = proj.projectDir.absolutePath
 
-        assert pDir + '/build/j2objcOutputs/lib' == ext.getDestLibDirFile().absolutePath
-        assert pDir + '/build/j2objcOutputs/src/main/objc' == ext.getDestSrcDirFile('main', 'objc').absolutePath
-        assert pDir + '/build/j2objcOutputs/src/test/objc' == ext.getDestSrcDirFile('test', 'objc').absolutePath
-        assert pDir + '/build/j2objcOutputs/src/main/resources' == ext.getDestSrcDirFile('main', 'resources')
-                .absolutePath
-        assert pDir + '/build/j2objcOutputs/src/test/resources' == ext.getDestSrcDirFile('test', 'resources')
-                .absolutePath
+        assert proj.file('build/j2objcOutputs/lib').absolutePath ==
+               ext.getDestLibDirFile().absolutePath
+        assert proj.file('build/j2objcOutputs/src/main/objc').absolutePath ==
+               ext.getDestSrcDirFile('main', 'objc').absolutePath
+        assert proj.file('build/j2objcOutputs/src/test/objc').absolutePath ==
+               ext.getDestSrcDirFile('test', 'objc').absolutePath
+        assert proj.file('build/j2objcOutputs/src/main/resources').absolutePath ==
+               ext.getDestSrcDirFile('main', 'resources').absolutePath
+        assert proj.file('build/j2objcOutputs/src/test/resources').absolutePath ==
+               ext.getDestSrcDirFile('test', 'resources').absolutePath
     }
 
     @Test
     void testFinalConfigure_MacOSX() {
-        Utils.fakeOSName = 'Mac OS X'
+        Utils.setFakeOSMacOSX()
         J2objcConfig ext = new J2objcConfig(proj)
 
         assert !ext.finalConfigured
@@ -82,8 +85,8 @@ class J2objcConfigTest {
     }
 
     @Test
-    void testFinalConfigure_nonMacOSX_translateOnlyMode() {
-        Utils.fakeOSName = 'Windows'
+    void testFinalConfigure_LinuxTranslateOnlyMode() {
+        Utils.setFakeOSLinux()
         J2objcConfig ext = new J2objcConfig(proj)
         assert !ext.finalConfigured
         ext.translateOnlyMode = true
@@ -92,10 +95,35 @@ class J2objcConfigTest {
         assert ext.finalConfigured
     }
 
-    // This protect against trying the native compile on a nonMacOSX system
     @Test
-    void testFinalConfigure_nonMacOSX_unsupported() {
-        Utils.fakeOSName = 'Windows'
+    void testFinalConfigure_WindowsTranslateOnlyMode() {
+        Utils.setFakeOSWindows()
+        J2objcConfig ext = new J2objcConfig(proj)
+        assert !ext.finalConfigured
+        ext.translateOnlyMode = true
+
+        ext.finalConfigure()
+        assert ext.finalConfigured
+    }
+
+    // This warns against doing a native compile on a nonMacOSX system
+    @Test
+    void testFinalConfigure_LinuxIsUnsupported() {
+        Utils.setFakeOSLinux()
+        J2objcConfig ext = new J2objcConfig(proj)
+
+        expectedException.expect(InvalidUserDataException.class)
+        expectedException.expectMessage('Mac OS X is required for Native Compilation of translated code')
+
+        assert !ext.finalConfigured
+        ext.finalConfigure()
+        assert ext.finalConfigured
+    }
+
+    // This warns against doing a native compile on a nonMacOSX system
+    @Test
+    void testFinalConfigure_WindowsIsUnsupported() {
+        Utils.setFakeOSWindows()
         J2objcConfig ext = new J2objcConfig(proj)
 
         expectedException.expect(InvalidUserDataException.class)