Skip to content

Commit

Permalink
Fix Windows Unit Tests - file/path separators
Browse files Browse the repository at this point in the history
- Fix all Windows unit tests (fixes #402)
- Small number of unit tests skipped on Windows, marked with TODOs

Testing:
- Fake OS specific file and path separators
- Explicit setFakeOS methods, e.g. setFakeOSWindows()
- Default to using native OS rather than fakeOSName for most unit tests
- Small number of unit tests use fakeOSName
- ‘inTestMustFakeOS’ setting removed
  • Loading branch information
brunobowden committed Aug 25, 2015
1 parent 14f82ba commit c74d3ef
Show file tree
Hide file tree
Showing 21 changed files with 575 additions and 204 deletions.
28 changes: 28 additions & 0 deletions .idea/codeStyleSettings.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ test {
// of just pointing to an HTML file.
exceptionFormat = 'full'
}
systemProperty 'inTestMustFakeOS', 'true'
}

pluginBundle {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,37 +60,73 @@ 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 String fakeOSName = ''
static void setFakeOSWindows() {
fakeOSName = 'Windows'
}

// Unset fake os, should be needed for @Before method
@VisibleForTesting
static String getLowerCaseOSName() {
static void setFakeOSNone() {
fakeOSName = ''
}

@VisibleForTesting
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')
}

@VisibleForTesting
static final String windowsAbsolutePathPrefix = 'C:'

static boolean isWindowsNoFakeAbsolutePath(String path) {
return isWindowsNoFake() && path.startsWith(windowsAbsolutePathPrefix)
}

static void requireMacOSX(String taskName) {
if (!isMacOSX()) {
throw new InvalidUserDataException(
Expand All @@ -99,6 +135,29 @@ class Utils {
}
}

// Same as File.separator but can mocked using fakeOSName
static String fileSeparator() {
if (isWindows()) {
return '\\'
} else {
return '/'
}
}

// Same as File.pathSeparator but can mocked using fakeOSName
static String pathSeparator() {
if (isWindows()) {
return ';'
} else {
return ':'
}
}

static String osTestCompatPath(String path) {
// Replace the native OS separator char with the possibly faked separator
return path.replace(File.separator, fileSeparator()).replace('/', fileSeparator())
}

static void throwIfNoJavaPlugin(Project proj) {
if (!proj.plugins.hasPlugin('java')) {
String message =
Expand Down Expand Up @@ -159,6 +218,14 @@ class Utils {
}

result = localProperties.getProperty(PROPERTY_KEY_PREFIX + key, null)

// // DO NOT COMMIT
// throw new InvalidUserDataException(
// "key: $key\n" +
// "result: $result\n" +
// "streamed localProperties: $localProperties\n" +
// "read localProperties: ${localPropertiesFile.readLines().join('\n')}"
// )
}
if (result == null) {
// debug.enabled becomes J2OBJC_DEBUG_ENABLED
Expand All @@ -172,8 +239,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 homePath = getLocalProperty(proj, 'home')
if (homePath == 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" +
Expand All @@ -186,15 +253,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 homeFile = new File(homePath)
if (!homeFile.exists()) {
String message = "J2OjcC Home directory not found, expected location: ${homePath}"
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 homeFile.absolutePath
}

// Reads properties file and arguments from translateArgs (last argument takes precedence)
Expand Down Expand Up @@ -297,7 +362,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -208,10 +208,11 @@ class XcodeTask extends DefaultTask {
if (path.endsWith('/')) {
throw new InvalidUserDataException("Path shouldn't end with '/': $path")
}
if (relative && path.startsWith('/')) {
boolean relativePath = path.startsWith('/') || Utils.isWindowsNoFakeAbsolutePath(path)
if (relative && relativePath) {
throw new InvalidUserDataException("Path shouldn't be absolute: $path")
}
if (!relative && !path.startsWith('/')) {
if (!relative && !relativePath) {
throw new InvalidUserDataException("Path shouldn't be relative: $path")
}
if (path.endsWith('*')) {
Expand Down
Loading

0 comments on commit c74d3ef

Please sign in to comment.