Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Windows Unit Tests All Fixed (fix #402) #404

Merged
merged 1 commit into from
Aug 26, 2015
Merged

Windows Unit Tests All Fixed (fix #402) #404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build.gradle
View file
Open in desktop
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'
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remind me why it is ok to remove this.
Are we no longer testing the same functionality on all OS-es (in the unit tests), up to path separators?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very hard to completely mock the OS.... including for example classes like java.io.File. I think a better approach is to use the fully native calls for as many of the unit tests as possible. Indeed, that's how someone will use the plugin. With passing builds on multiple OSs, we automatically get good coverage.

There are a small number of cases where you'd like to prevent breaking another OS. So for the tasks that only run on OS X, it uses setFakeOSMacOSX() and runs those tests. For most tasks it's better to get the coverage on the different systems.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's fair. now that we have hermetic Linux + Win + OSX builds gating the merge, I'm less worried about this.


pluginBundle {
Expand Down
6 changes: 3 additions & 3 deletions src/main/groovy/com/github/j2objccontrib/j2objcgradle/J2objcConfig.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/CycleFinderTask.groovy
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/PackLibrariesTask.groovy
View file
Open in desktop
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
28 changes: 13 additions & 15 deletions src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/TestTask.groovy
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/TranslateTask.groovy
View file
Open in desktop
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
93 changes: 69 additions & 24 deletions src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/Utils.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 =
Expand Down Expand Up @@ -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" +
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 10 additions & 7 deletions src/main/groovy/com/github/j2objccontrib/j2objcgradle/tasks/XcodeTask.groovy
View file
Open in desktop
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 @@ -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
Expand Down
64 changes: 46 additions & 18 deletions src/test/groovy/com/github/j2objccontrib/j2objcgradle/J2objcConfigTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down
Loading