Forem: Kostas Tsalikis

A confusing build failure

Kostas Tsalikis — Wed, 06 Nov 2019 23:00:09 +0000

TL;DR

Do not overuse mock test doubles.
Check the quality of tests in code reviews as well.
Validate that the build between Android Studio and CI servers is invoked through the same commands.
Investigate thoroughly build failures and try to have them fixed as soon as possible, before merging.

A wild failing build appears

Nowadays, it has become quite a common practice to build and check software on machines other than local workstations, most of the time called CI servers(CI stands for Continuous Integration). This can be done either manually or automatically, usually triggered by a pull/merge request or a direct push on the main trunk (in git terminology most of the times named master).

The point is that there’s a single place where binaries are packaged and code is checked for quality and correctness, so we can merge changes safely and with confidence.

When a failure appears in a build, it should be investigated why it happened. Did a commit actually break functionality? Were static code analysis checks too harsh and maybe they must be relaxed a little bit? Recently, one of the failures that left me most perplexed was a build where the unit tests failed on the CI server but passed locally! How did this happen? Investigating on this confusing build failure unearthed bigger problems than just a breaking unit test.

Mockito cannot mock final classes

So why was the build failing? When the unit tests ran, some of them would fail with an error stating that Mockito (more specifically mockito-kotlin) - a library commonly used in Java and Android to provide mock test doubles for behavior verification - cannot mock final classes.

In order to understand the error, let’s try to understand first how Mockito works. It creates, at runtime, objects that extend interfaces or classes designed to verify that a set of methods were called with the intended parameters. If a class that is attempted to be mocked is declared as final, then the error above will be thrown as a final class cannot be extended at compile or run time. In our unit test, a Kotlin class was mocked and as Kotlin classes are by defaultfinal, unless explicitly declared open, the error above was thrown.

Although the culprit is found, we still have not figured out why the tests passed locally but failed on the CI server.

Maybe Mockito can mock final classes after all

If Mockito cannot mock final classes, then why do local tests pass? Well, sure there must be some way to mock a final class, and that’s exactly what happens when the org.mockito:mockito-inline dependency is added to the gradle build file.

The problem is that the failing module did not compile with the dependency above. Instead, a submodule did declare mockito-inline on its build.gradle but not as a transitive dependency, meaning that final classes could be mocked on that submodule only, not on the top module whose tests were failing.

This could easily be fixed by either declaring mockito-inline as transitive, or adding it on the top module, but that would not explain why the test resources are compiled differently on the CI server than locally.

Based on gradle files, on the CI server the build failed as expected, but locally mockito-inline was compiled on the top module (where it shouldn’t) and it wrongfully passed. This, however, begs the following question: how can the CI server be compiled and tested in a different way than the local build?

It turns out that different commands are employed to compile and test the source code. On the CI server the project is compiled and tested with gradle commands, thus the mockito-inline is not compiled and the test that mocks Kotlin classes fails. On the other hand, the local builds used the test runner that is natively built into Android Studio, which happens to compile all libraries irrespectively of how they are declared on each module’s build.gradle.

But why did mockito-inline was used on the first place?

At last, the mystery has been solved but let’s dig a little deeper. Why did a test try to mock an otherwise concrete class? And when/why was the mockito-inline dependency introduced first?

The Kotlin class was mocked just as a filler object for the constructor. No verifications were performed on that mocked object - neither was it used to stub a value. Hence, the concrete class should be used as is or in case of an interface, which cannot be instantiated, a dummy test double. If, however, a concrete class was mocked because it was difficult to instantiate (e.g. it has too many dependencies on its constructor), then this is a moment where one needs to pause and think twice. As for value(data) classes?

Quoted directly from the book Growing object-oriented Guided by Tests [¹]:

Don’t Mock Values

On the other hand, mockito-inline was introduced in the submodule in order to verify the behavior of a third-party library. Let’s say that we had to test the persistence layer of an app. We could either run an integrated test in order to check if the code we’ve written works well against the real database, or we could mock the database as below.

@Test
    public void givenDbIsEmpty_whenInsertData_thenDbSavesTheNewEntry() throws IOException {
        /* DAO: Data Access Object
         * usually referred to an object that fetches 
         * data from a database or other kinds of data source. */
        SomeDAO aDao = new SomeDAO();
        database = mock(ExternalDatabase.class); 
        when(database.select(any())).thenReturn(new SomeDAO());

        database.insert(aDao);

        SomeDAO savedDao = database.select(aDao);
        assertThat(selectedDao, is(equalTo(aDao);
    }

First of all, the problem with this test is that it actually tests nothing. The class under test is mocked and its result is stubbed, so it seems all that is tested here is the ability of the mock framework to stub a value. These kinds of tests give us no feedback and no confidence, in other words they provide zero value. We should instead automate the test for the real thing or test it manually.

What’s more, we are not listening to the tests. Instead of trying to guess how the library should respond to a request and find ways to mock/stub whatever is required to feed that answer to an assertion, we should add an Adapter layer over the third-party code, test the Adapter with the library and mock callbacks that we may pass to the Adapter layer.

Perhaps some direct quotes can paint the picture in a better way [²]:

The mess in such tests is telling us that the design isn’t right but, instead of fixing the problem by improving the code, we have to carry the extra complexity in both code and test.

A second risk is that we have to be sure that the behavior we stub or mock matches what the external library will actually do.

Mitigate the many faces of the problem

So far we’ve seen that there is a build failure that was only caught on the CI server caused by the abuse of mocks on unit tests and the difference in the way CI and local builds are created. Moreover, the failure managed to pass to the main trunk and obstruct other pull requests even though no bugs were introduced. Is there a single root cause in this situation? Multiple layers of safety had to be overridden in order for the failure to reach the mainline.

This is not so much an issue of technical deficiency - all developers on the team are quite competent. In order to understand the problems and their various causes, let’s state the facts and try to analyze the situation.

Mocks are used excessively and on the wrong context. They are a special case of test doubles that are used for verification testing [³].

Using mocks as the de facto test double highlights a lack of understanding of the tools used for testing. Quoting again Nat Pryce and Steve Freeman [¹]:

our intention in test-driven development is to use mock objects to bring out relationships between objects.

In every test, it should be carefully evaluated what kind of test doubles (if any) are required and try to minimize mocks only for verifying behavior on interfaces. Of course, there are times where every rule needs to be broken, but most of times we can avoid using tools such as mockito-inline and PowerMock.

The build failed too late and only on the CI server. The local workstations and the CI server should build with the same commands in order to avoid unpleasant surprises like this one. Unfortunately, it may be a common assumption that Android Studio builds and tests the same way with the setup of a CI server(this surely caught me by surprise as well).

Maybe we could check once in a while the way Android Studio builds because an update can change how things are built and tested.

Broken code passed on the main trunk. Even though the build failed, eventually the code was merged and made available for others.

The safety nets here could be more thorough code reviews that check the quality of the tests as well.

Summing up

Sometimes being in a hurry does not help and if the CI server is erratic and fails often from timeouts, we may think that it’s a fault negative. In any case, we should always investigate why builds fail and try to fix them again as soon as possible. Just having tests is not enough - they should provide fast feedback and confidence. They should be fast, deterministic, easy to read and understand, easy to set up and their failures clear to diagnose. These qualities of tests are the weapons that a developer can exploit in order to refactor aggressively and keep the system easy to understand and change. Add critical thinking to the mix in order to listen to what tests are trying to tell us about our design and we have the ingredients to battle complexity.

This may sound a little provocative, but we could as well delete tests that do not adhere to the qualities above as we end up with double the burden of maintenance and confusion.

As a personal critique on my part, I could probably say that when I was tasked to provide guidelines to the team about verification testing, I felt that I should have emphasized even more on the dangers of mocking too much. After all one cannot learn just by one example, especially from code that is not created with testing in mind - meaning there will be cut corners and “broken” rules on these examples.

Finally, this can be seen as a further proof that tools by themselves do not solve problems. Having tests and a CI server is not enough. It is critical to have a deep understanding why these set of techniques and tools are used, as well as the right place and way to use them. Fortunately, this level of understanding is not the result of arcane magic but can be acquired through repetitive practice and retrospection. We are bound to fail through inexperience but bound to succeed through curiosity and will to learn.

Special and many thanks to Kleomenis Tsiligkos and Michalis Kolozoff for proof reading.

Nat Pryce - Steve Freeman: Growing object-oriented Guided by Tests, Chapter 20: Listening to the tests^[return]
Nat Pryce - Steve Freeman: Growing object oriented Guided by Tests, Chapter 8: Building on Third-Party Code^[return]
Martin Fowler: Mocks aren’t Stubs^[return]

Uploading an Android library on JCenter

Kostas Tsalikis — Tue, 21 May 2019 19:47:02 +0000

Facing a lack of inspiration for quite some time I decided to draw some from an old project of mine. A couple of years ago I created an open-source library named target-layout which was basically a FrameLayout that draws a view on its center and some drawables around it, letting the user pinch zoom on the view and select through a range of levels.

Of course a GIF might be the best way to showcase what the library does.

Apart from the joy of creating a custom and unique UI and delving into the depths of how a view is measured, laid-out and drawn I decided to learn how to publish the library on JCenter. Unfortunately, I haven’t been the world’s best maintainer, so I left the library targeting old Android SDK versions, without any support for AndroidX. What’s more, I had left the project without Continuous Integration (CI from now on) and as the years passed, I forgot how publishing on JCenter works, so I decided to revisit these topics before doing any further work with the library itself.

Adding CI on your open-source library

The first think I had to do was updating all its outdated dependencies. This meant updating gradle versions along with target and compile Android SDK versions so any new users of the library would be able to compile it with no problems and warnings. Even though I am the maintainer and could push directly to the master branch, I decided to go for a pull request process with myself. Doing so meant that I would emulate the process of someone else trying to use and modify the library, so I needed to add some checks from an external CI server in order to verify that the library actually builds.

Among the most usual CI services that support open-source projects and are Android friendly are CircleCI and TravisCI. As I had some previous experience with TravisCI on a toy project, I decided to use TravisCI in order to kick-start the process of making the library modernized. In order to start using TravisCI. First of all, it is needed to create an account or sign in with your GitHub account. After doing so, we can add from the dashboard a git repository and then add a .travis.yml file on the root of the repo. The .travis.yml is essential as it instructs TravisCI on how to build the project - if TravisCI does not find the .travis.yml on the root of the repo it will be unable to build. Below you can find a sample .travis.yml for building an Android project:

language: android
jdk: oraclejdk8

before_cache:
  - rm -f $HOME/.gradle/caches/modules-2/modules-2.lock
  - rm -fr $HOME/.gradle/caches/*/plugin-resolution/

cache:
  directories:
    - $HOME/.gradle/caches/
    - $HOME/.gradle/wrapper/

env:
  global:
    - ANDROID_API=28
    - ANDROID_BUILD_TOOLS=28.0.3

android:
  components:
    - tools
    - platform-tools
    - build-tools-$ANDROID_BUILD_TOOLS
    - android-$ANDROID_API
    - extra-google-m2repository
    - extra-android-m2repository # for design library

licenses:
  - android-sdk-preview-license-.+
  - android-sdk-license-.+
  - google-gdk-license-.+

before_install:
  - mkdir "$ANDROID_HOME/licenses" || true
  - echo -e "\n8933bad161af4178b1185d1a37fbf41ea5269c55" > "$ANDROID_HOME/licenses/android-sdk-license"
  - echo -e "\n84831b9409646a918e30573bab4c9c91346d8abd" > "$ANDROID_HOME/licenses/android-sdk-preview-license"
  - yes | $ANDROID_HOME/tools/bin/sdkmanager "build-tools;24.0.3"
  - chmod +x gradlew

script:
  - "./gradlew :library_module_name:clean :library_module_name:build"

Explaining the .travis.yml

The yml above instructs the CI service to clear the caches before building and exports as environment variables the desired Android API and build tools versions. We then declare which components we will use, along with their licenses. This is something that, in our daily work life, do through Android Studio manually but since we are building on a CI server, we need to do this programmatically. By declaring them this way on the .travis.yml we are instructing TravisCI to download the desired components and accept any of the licenses needed. As there are some problems with accepting the licenses, we additionally instruct to accept the licenses through the sdkmanager tool and finally make the gradlew file executable in order to let TravisCI execute the gradle scripts for building the project. Finally the script section is where the build actually happens. Since we are only interested in building the library module and not the sample that accompanies it, we only clean and build the library module(on the script with a name exampled as library_module_name)

Some pain points

We must be sure that we have added the .travis.yml on the root of the git directory as otherwise TravisCI will not be able to build. Another pain point is the licenses. Accepting them through the sdkmanager solves any persisting problems. Unfortunately I did not find a way to access the lint html report on TravisCI, so I ended up showing any reports as text on the console through the build.gradle of the library module.

android {
    compileSdkVersion 28
    buildToolsVersion "28.0.3"

    lintOptions {
        textReport true
        abortOnError true
        warningsAsErrors false
    }
    ...
}

Updating gradle versions and moving on AndroidX

At this point we have a build that passes through TravisCI and we are ready to update gradle and Android SDK versions. Fortunately this is done easily through Android Studio since it already gives warnings on the library’s build.gradle file. Just hitting Alt+Enter on the respective versions, updates them automatically and the migration tool for AndroidX works fine as the project does not have any dependencies apart from the Android appcompat library - so I didn’t have to deal with any of the problems of the AndroidX migration. Now we are ready to create the pull request, see it pass the TravisCI build and safely accept it. What’s more we can add the TravisCI build badge in our README.md to showcase that the build is passing.

Uploading on JCenter

Now that the build passes, we are finally ready to upload the library’s artifacts on JCenter so everyone will be able to add it as a dependency on their builds. But before uploading anything anywhere, first we need to know what files to actually upload!

The maven repository

For our library to be traceable and downloadable we need to upload it on a maven repository. A very good and thorough explanation of what a maven repository is and how it works can be found here. In a few words, a maven repository is a directory where all the library’s executable files are stored and are available to download. The path of the directory is determined by the library’s .pom file, which essentially gives the library its name as well and adds any transitive dependencies that it has. The .pom file for the target-layout library that serves as an example for this post is the following:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <modelVersion>4.0.0</modelVersion>
  <groupId>com.tsalik.targetlayout</groupId>
  <artifactId>targetlayout</artifactId>
  <version>1.0.2</version>
  <packaging>aar</packaging>
  <dependencies>
    <dependency>
      <groupId>androidx.appcompat</groupId>
      <artifactId>appcompat</artifactId>
      <version>1.0.2</version>
      <scope>runtime</scope>
    </dependency>
  </dependencies>
</project>

The above means, would anyone like to fetch the library from a maven repository with gradle, they would have to add in the dependencies of their build.gradle file the following line:

implementation "com.tsalik.targetlayout:targetlayout:1.0.2"

Since this is an Android library, it is declared that an .aar(named targetlayout-1.0.2.aar more specifically) file should be downloaded, along with the dependencies of the library, which is the androidx.appcompat:appcompat:1.0.2.

Now we know that we need to produce the .aar from the code of the library and add it along with a .pom file, including any extra resources or javadoc on a maven repository, so next steps should be about building the library and adding any of the artifacts above on a specific maven repository (in our case we want to eventually upload it on the JCenter repository)

Let’s build and publish locally

First of all, we need to create the artifacts mentioned above on a local repository. To do this we need to know how to publish a maven repository through gradle, which is the de facto tool for building on Android (although alternatives like Bazel and Buck do exist). An extensive documentation on how maven publishing works can be found here(at the time of writing for the 5.4.1 version of gradle). Although the official gradle documentation describes how publishing works the bintray plugin documentation(version 1.8.4 at the time of writing) is more specific on how to publish specifically in Android and avoid common pitfalls.

Below you can find the example gradle script for publishing the target-layout library:

apply plugin: 'maven-publish'

task androidJavadocs(type: Javadoc) {
    failOnError = false
    source = android.sourceSets.main.java.srcDirs
    ext.androidJar = "${android.sdkDirectory}/platforms/${android.compileSdkVersion}/android.jar"
    classpath += files(ext.androidJar)
    exclude ' **/R.html', '** /R.*.html', '**/index.html'
}

task androidJavadocsJar(type: Jar, dependsOn: androidJavadocs) {
    classifier = 'javadoc'
    from androidJavadocs.destinationDir
}

task androidSourcesJar(type: Jar) {
    classifier = 'sources'
    from android.sourceSets.main.java.srcDirs
}

project.afterEvaluate {
    publishing {
        publications {
            targetlayout(MavenPublication) {
                groupId project.ext.PUBLISH_GROUP_ID
                artifactId project.ext.PUBLISH_ARTIFACT_ID
                version project.ext.PUBLISH_VERSION

                artifact bundleReleaseAar
                artifact androidJavadocsJar
                artifact androidSourcesJar

                pom.withXml {
                    final dependenciesNode = asNode().appendNode('dependencies')

                    ext.addDependency = { Dependency dep, String scope ->
                        if (dep.group == null || dep.version == null || dep.name == null || dep.name == "unspecified")
                            return // ignore invalid dependencies

                        final dependencyNode = dependenciesNode.appendNode('dependency')
                        dependencyNode.appendNode('groupId', dep.group)
                        dependencyNode.appendNode('artifactId', dep.name)
                        dependencyNode.appendNode('version', dep.version)
                        dependencyNode.appendNode('scope', scope)

                        if (!dep.transitive) {
                            // If this dependency is transitive, we should force exclude all its dependencies them from the POM
                            final exclusionNode = dependencyNode.appendNode('exclusions').appendNode('exclusion')
                            exclusionNode.appendNode('groupId', '*')
                            exclusionNode.appendNode('artifactId', '*')
                        } else if (!dep.properties.excludeRules.empty) {
                            // Otherwise add specified exclude rules
                            final exclusionNode = dependencyNode.appendNode('exclusions').appendNode('exclusion')
                            dep.properties.excludeRules.each { ExcludeRule rule ->
                                exclusionNode.appendNode('groupId', rule.group ?: '*')
                                exclusionNode.appendNode('artifactId', rule.module ?: '*')
                            }
                        }
                    }

                    // List all "compile" dependencies (for old Gradle)
                    configurations.compile.getDependencies().each { dep -> addDependency(dep, "compile") }
                    // List all "api" dependencies (for new Gradle) as "compile" dependencies
                    configurations.api.getDependencies().each { dep -> addDependency(dep, "compile") }
                    // List all "implementation" dependencies (for new Gradle) as "runtime" dependencies
                    configurations.implementation.getDependencies().each { dep -> addDependency(dep, "runtime") }
                }
            }
        }

        repositories {
            maven {
                // change to point to your repo, e.g. http://my.org/repo
                url = "$buildDir/repo"
            }
        }
}

Although it seems daunting, let’s try to dissect it. First of all, we declare our own tasks for generating Javadoc and Android resources on their respective jars. The publishing and publication closures are from the maven-publish plugin, which we must apply on top of the script. Inside the publication closure we declare a maven publication named targetlayout - named after the library (for your own library you must apply an appropriate name).

Continuing inside the publication closure, we apply the groupdId, artifactId and version in order to name the library appropriately. The values on the example above are set as extra project properties on the build.gradle file of the library’s module so that we can reference them in one single point. Then we declare which artifacts (i.e. files) we need the publication to have. We want the .aar for the release build, as well as the Javadoc and any extra resources as jars (we do so by calling the tasks we declared above).

Finally, we manually add all the dependencies of the library for compile, api and implementation configurations, as otherwise it will not be done automatically (the bintray-gradle-plugin explains nicely why).

In order to check if all this configuration for the publication works, we can add the repositories closure and publish on a local directory (in this case a directory named repo under the library module’s build folder). Now in order to test that everything is fine we can execute:

./gradlew clean :targetlayout:publish

and we should be able to check that all the artifacts(aar, javadoc, pom files) have been published on the build folder and the pom has the right groupId, artifactId and version.

So where should we upload then?

Now that we have seen how a publication works and what files it contains, we are ready to finally upload the library on a maven repository other than the local one that we created. Among the various public maven repositories that exist, the more popular ones for Android development are Maven Central, JCenter and JitPack. A comparison between them is out of the scope of this post and since I had already uploaded the library on JCenter, we will focus only on uploading there. Fortunately, there already exists a gradle plugin for uploading on JCenter so there will be no need to re-invent the wheel.

In order to install the bintray gradle plugin, we apply the classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.4' on the dependencies of the top build.gradle of the project and then apply it on the same script declared above(which is responsible for publishing).

apply plugin: 'maven-publish'
apply plugin: 'com.jfrog.bintray'

...
project.afterEvaluate {
    publishing {
        publications {
            targetlayout(MavenPublication) {
                ...
            }
        }
    }

    bintray {
            Properties localProperties = new Properties()
            if (project.rootProject.file('local.properties').exists()) {
                localProperties.load(project.rootProject.file('local.properties').newDataInputStream())
            }
            user = localProperties.getProperty("bintray.user")
            key = localProperties.getProperty("bintray.apikey")
            pkg {
                repo = 'target-layout'
                name = 'target-layout'
                licenses = ['MIT']
                vcsUrl = 'https://github.com/tsalik/target-layout.git'
                version {
                    name = "1.0.2"
                    released = new Date()
                }
            }
            publications = ['targetlayout']
        }
}

There are a couple of things to notice here. First of all, we must have already been singed in JCenter and created a maven repository. I will leave this part out and provide references on how to do it. You can check that the repository and the name of the library have the same name - target-layout. This is not something mandatory, it just happened at the time I was creating the repository.

In order to be able to upload the artifact of the publishing, we must somehow authorize ourselves with bintray. The user and key fields just do that - but be aware. We MUST NOT add these values on version control as everyone could check these values and upload anything, they want on your maven repository. We can add them as environment variables if we publish and upload through the CI server, or add them on local.properties(which MUST be on the .gitignore).

If you decide to inject them through the local.properties, make sure that you check that the file exists - in a CI server it will not and the build will fail.

Note that although I have declared the version as an external property, instead of referencing it I have hardcoded it. I’m sure on the future there will be some frustration with my past self unless I fix it soon 😅.

Finally, after adding any required metadata, we declare which publications we want to upload. This must be the same value with the name that we declared on the publishing section. At last we are ready to publish the library from the command-line:

./gradlew clean :targetlayout:bintrayUpload

Final thoughts

Ultimately revisiting how to add CI on an open-source project and automating its publishing process proved to be a good exercise. Apart from the frustration of some (a lot) broken builds due to misconfigurations on the travis.yml for setting up CI it was an overall enjoyable ride. Having to deal with how a publication works, clarified a lot of things on how maven works and highlighted that creating a publication with its artifacts and uploading it on a public maven repo are actually two different and partially unrelated things.

Just publishing locally would have been enough as I would be able to manually upload the artifacts as I did in the past, but this time instead of being dependent on someone else’s ready-made solution, I decided to actually check out how to do this without the help of a third-party plugin. After all, the lesser the plugins the lesser the pain of building a library or app. The bintray plugin was indeed invaluable as it made easy the upload part of publishing the library.

Next Steps

Now that the library is updated with the latest gradle and Android SDK versions I can investigate more thoroughly some lint checks and maybe consider adding some animations when the layout first attaches on the window. It is amazing that what used to be on the brink of deprecation now has a new spark and gave inspiration to investigate a variety of things. Another step for future work is to add gpg signing and push the library on Maven Central as well.

References

Although there are a lot of posts that cover the publication that have far more detail on how to setup your TravisCI account and how to create an account and the repository on JCenter I decided to cover more in depth on how a publication works on a maven repository.

What’s more, most of the posts used other gradle plugins for the publishing part on top of gradle’s maven-plugin and since the scope of the exercise was to check how it works with minimal dependencies, I decided to depend only on what’s vanilla on gradle.

The posts from Anitaa Murthy, Yegor Zatsepin and Wajahat Karim acted as guides for this one, with excellent replication steps on how to set up the repository on JCenter.

Special thanks to Michalis Kolozoff for proofreading

Kotlin puzzles

Kostas Tsalikis — Sun, 27 Jan 2019 20:55:38 +0000

Some context before the puzzles

One of the books that I’ve be meaning to finish is Growing Object-Oriented Guided by Tests written by Steve Freeman and Nat Pryce. In their excellent book the authors not only describe how to use tests in order to drive the design of a system but also provide a comprehensive example where they showcase in practice what they preach.

The example is a Swing application which receives and sends events to a XMPP(a publish/subscribe protocol) server and uses some dated testing libraries(Windowlicker for testing the Swing GUI and JMock for testing with mocks). Even though all code is in Java, I decided to minimize the use of Kotlin in order to focus on the techniques the authors provide and added Mockito instead of JMock for testing mocks. Finally, I ended up with a mixed codebase with most parts written in Java, adding some minimal Kotlin flavor in four cases:

data classes to replace the use of Apache’s commons equals/hashcode for creating immutable objects.
sealed classes instead of enums
more idiomatic Kotlin features like map to replace for loops.
computed properties instead of functions inside some Kotlin code.

In general I wanted to avoid extensive use of Kotlin in order to avoid unpleasant surprises and yet surprises happened, particularly in numbers 3 and 4 from the list above.

Puzzle Number 1: a problem with split

In their example, the authors have a class which parses a CSV like response from the network. The response looks something like this:

"Event: PRICE; Price: 100; Increment: 20; Bidder: Someone;"

and they parse the response with the snippet of code below:

private Map<String, String> unpackEventFrom(Message message) {
        Map<String, String> event = new HashMap<>();
        for (String element : message.getBody().split(";")) {
            String[] pair = element.split(":");
            event.put(pair[0].trim(), pair[1].trim());
        }
        return event;
    }

In this snippet, they just split the string first on ; and then create pairs with splitting : finally they put keys and values in a map, pretty straightforward. This kind of code is something that could use some Kotlin flavor, so lets rephrase it:

messageBody.split(";")
        .associateTo(auctionEvent.fields) {
                val pair = it.split(":")
                pair[0].trim() to pair[1].trim()
        }

The Kotlin snippet splits the message on ; then creates a pair by splitting on : and associates the pair to an empty map(the auctions.fields val that is omitted for brevity). The code was written in Java first, the tests passed but a surprise was there when I converter it to Kotlin, and specifically an IndexOutOfBounds one! How oh how did this happen? It seems that Kotlin’s split function has a different implementation than Java. Something that I was not aware of is that Java’s split also has a limit parameter, and when calling split(;) this actually calls split(;, 0). Koltin does also the same but the difference is how zero is interpreted on the two languages.

Quoted directly from Java’s documentation:

If limit is zero then the pattern will be applied as many times as possible, the array can have any length, and trailing empty strings will be discarded.

We can see that Java applies an extra meaning to the limit parameter. On the contrary, Kotlin let’s the user apply only non-negative limits and does not discard the trailing empty spaces, unless you explicitely tell it to do.

The exact equivalent in Kotlin would be something like this:

messageBody.split(";")
        .dropLastWhile { it.isEmpty() }

Why would Kotlin’s creators do something like this? One could argue that Java is the one which is wrong here, since the limit parameter is not only specifying the number of times that a delimiter should apply, but also adds a different meaning that is completely irrelevant and treats zero like a special value. Another nice explanation of why Kotlin’s split is different from the Java one can be found on this stackoverflow answer.

Puzzle Number 2: This is not the (computed) property you’re looking for

The snippet with the parsing was from an Event class. The authors of the example decided not to expose the map but instead create methods in order to expose the values of an Event.The unpack method from above acted as a Factory Method and initialized the map and the various methods exposed the values of the event. In my Kotlin flavored Event I created a companion object and private constructor to emulate the Factory method and decided to use computed properties. At first I tried something like this:

class Event private constructor() {

        private val fields = mutableMapOf<String, String>()

        val type = parse("Event")

        private fun parse(key: String) = fields.get(key)!!

        companion object {
            fun from(message: Message): Event {
                val auctionEvent = Event()
                messageBody.split(";")
                    .dropLastWhile { it.isEmpty() }
                    .associateTo(auctionEvent.fields) {
                        val pair = it.split(":")
                        pair[0].trim() to pair[1].trim()
                    }
                return auctionEvent
            }
        }
    }

Alas, I could not be more wrong on thinking that the property type would actually parse the key from the map after the map was populated with values on the from function. Since the Event is created before populating the map, the type property will parse immediately the key from the map, and since the map will be empty on construction time, the value will be null and a glorious kotlin.KotlinNullPointerException will be thrown, as the parse method gets the key with the not null!! operator. The type field seems as a computed property, but it actually is not, it should obviously be declared as below:

val type
        get() = parse("Event")

Even though this surprise was entirely my own fault it got me thinking that maybe it would be better to eventually declare a function instead of a computed property. The use of computed properties surely is a more idiomatic use of Kotlin, but on the other hand I feel like a property should act as the state of the object and in this example the state of the object is the map and we just expose its values on transformations of the values of the map, which should be exposed functions. Of course I’m not advocating against the use of computed properties, this is just a rant from a personal failure.

Final thoughts

As we can see, even when we try to minimize the use of idiomatic Kotlin or just try to port Java code to Kotlin, easy mistakes can be made. Of course this can happen whenever we’re trying to learn a new language but porting Java to Kotlin is more devious, as even though the two languages seem to have similar API they can actually differ on their implementation. Caution is needed, but in the example that I presented I happened to be lucky as the use of TDD provided a nice regression suite that caught my faults immediately - and indeed they were my own faults as I made assumptions on the APIs and idioms without checking Kotlin’s documentation first!

Photo by Gabriel Crismariu on Unsplash

Git tip: cherry-pick with an -x

Kostas Tsalikis — Sat, 29 Sep 2018 10:23:36 +0000

TL;DR

cherry-pick takes a commit and applies its changes to your current branch.
When working with others, use -x option in order to highlight on the commit message that this is cherry-picked and not the original commit.
Use it sparingly, instead of a merge, when you need only one specific commit.
Overusing it can result in confusing history with duplicate commits, thus merge is generally preferred.

What is a cherry-pick?

When cherry-picking you basically take the commit that you’re targeting and apply its changes to your current HEAD. This will lead to a brand new commit with a different SHA and the same commit message.

git cherry-pick <target commit SHA>

Why cherry-pick in the first place?

Let’s say that you’ve drawn yourself into a corner – you need changes from another branch, but you don’t need all the changes, just some commits. Here is the only case where cherry-pick should be applied.

Main disadvantages

Most times developers choose to merge instead of performing a cherry-pick – but why is that? Well, one must be very careful when applying cherry-pick, especially with the order that different commits are applied. When merging, you can be sure that the commits are applied with the right order, so you just have to resolve appropriately the conflicts. On the contrary, when dealing with a number of cherry-picks, you must manually find the correct order of the commits and apply them one by one according to their date.

What’s more, you end up with duplicate commits, since the name of the cherry-picked commit will be the same with the original one.

So, cherry-pick is bad because:

You must manually apply the cherry-picks – depending on the number of commits that you have to apply it may be difficult to find the right order and this is surely an error prone process.
Duplicate commits result in weird looking and ugly history.
Let’s say someone else needs that fix in another branch, will they pick the right commit or will they cherry-pick the cherry-picked one. After all, the cherry-picked one may have a conflict that you do not need.

A contrived example of ugly history.

Below follows a contrived example of how a cherry-pick can make the history look ugly. Let’s say that you create two commits on master, A and B, and at that moment you want to create a new feature branch(named with an extra dose of imagination feature) and add a new commit C.

At the moment we have something like this.

A --- B <- (master)
          \
           \
            C <- (feature)

Then we go back again to master and commit D. We commit E on feature, but before that, we would need to apply fix D from master, thus we cherry-pick D.

   A --- B --- D     <- (master)
          \
           \
            C --- D' --- E     <- (feature)

At this point, we would like to merge the feature back to master. Now the graph would seem something like this:

A --- B --- D ----- Merge Commit     <- (master)
          \                /
           \              /
            C --- D' --- E     <- (feature)

And a git log --oneline would show us the duplicate commits:

5b5f739 (HEAD -> master) Merge branch 'feature'
d5c76ad E
52ed367 D
c097b61 D
af599fa C
383a1e1 B
c1fffad A

Of course, in this example we could use rebase in order to avoid duplicate commits, but one can think of a scenario where the second branch would be a long lived one and would need for only a hotfix to be applied from the upstream.

Working with others

So, what do you do in case that you are not the only one that needs these precious fixes? Naturally, the other person will try to search, with the commit message, the desired commit. Guess what, you will find more than one commit and you won’t know which one to apply. One could of course try to change the commit name so it can hint that it’s a cherry-pick commit and not the original one. This can be done with the following command:

git cherry-pick <target commit SHA> -e

This will open an editor where you can change the commit message and append something that hints that this is a cherry-pick commit.

After some digging I found that there is another option which automatically appends (cherry picked from commit <original commit's SHA>)

git cherry-pick <target commit SHA> -x

At least now you know which commit is cherry-picked or not and what was the original commit as well.

Cover image by Robert Zunikoff on Unsplash

Kotlin data vs open class

Kostas Tsalikis — Thu, 29 Mar 2018 19:45:19 +0000

The Kotlin data class

One of the many handy features that the Kotlin language offers is the data keyword. When we declare a class with the data keyword the compiler implements the equals(Object o),hashCode() and toString() methods, thus saving us from the trouble to do it manually. The official documentation provides comprehensive examples and nowadays it has become a widely known language feature, so an example can be left outside of the scope of this post.

All classes in Kotlin are by default final

Another thing that Kotlin does behind the scenes is to compile every class that you declare as final, unless you add the open keyword to it. We can see how the language is influenced from Effective Java 2nd Edition(Edition 3 is out as well!). More specifically, items 16 and 17 state the following:

Item 16: Favor composition over inheritance

Item 17: Design and document for inheritance or else prohibit it

A clash between keyword definitions

This is a very nice feature of the language, as it tries to protect us from the various perils of improper use of inheritance, as it is described in the book. Unfortunately, things begin to become messy when you try to declare a class both open and data. The compiler will complain that a data class cannot be at the same time open, giving us an error.

Let’s stop for a moment and consider why the architects of the language decided to impose such an obstacle. First of all, we will try to remember some wise words from the Growing object-oriented systems, driven by tests book from Steve Freeman and Nat Pryce. Chapter 2 presents a distinction between values and objects. Objects communicate between each other by sending messages and have some specific behavior depending on their state. If their state changes, so does their behavior. On the other hand values - to paraphrase - are just bags of data, used for computations.

Now that the definition and distinction between objects and values is clear, we can see that it does makes sense for Kotlin to impose such rules when using the data and open keywords. A data class is a value, holding only - preferably immutable - data without behavior. Since it makes sense to extend a class in order to change its behavior and the data class has no behavior, why should we be able to extend it?

Although the reasons for this design decision are quite valid, the imposed restrictions make the transition from Java to Koltin difficult. We may have in our codebase a class that’s a perfect candidate to become a data class, but if for some reason this class is extended by another class, you cannot make that transition instantly, as you have to fix first the fact that the class is extended. The problem becomes even worse when we have a deep nested hierarchy.

Some scenarios and possible ways to tackle the problem

One way to deal with this, is to check if the class has any fields or not. If it doesn’t have and it just acts as a marker, we could go and delete that class, trace the compiler errors and replace with the parent class. Indeed we can use this technique, provided that child class is not used in any instanceOf checks. In that case the problem becomes even worse especially when the parent class is involved in the instanceOf checks as well. Here we have a behavioural change in the system which we cannot fix just by replacing the check with the parent class, one that is very difficult to change. The sins of the past have finally caught up with us.

Now let’s consider the case where the child class extends the parent just to inherit some or all the fields of the parent and adds its own as well. Then we could try to use some kind of composition depending on the case.

Final thoughts

Maybe the nastiest of the problems described above is the instanceOf checks. If we could find a way to get past these checks problem, then replacing the children with the parent or passing the parent as a dependency becomes less difficult.

Unfortunately, this post does not provide a final solution, but rather tries to reason with Kotlin’s implementation decisions and explore some thoughts on how to tackle the problems that arise during a Java to Kotlin migration.

Architecture Decision Records

Kostas Tsalikis — Wed, 14 Feb 2018 12:42:41 +0000

In a previous post, we tackled the problem of doing a large change in your system incrementally. During refactoring, you can stumble upon a lot of problems, but one of the greatest is, how do you react to a past decision that you cannot understand? Do you accept it, with the risk of continuing to pay the technical debt associated with it? Or do you discard it, with the risk of losing some important semantics that should’t be lost along the way?

If you’re lucky enough, the person that made that decision will be one or two desks away. On the other hand, there is a high probability that the decision above was made a long time ago, and that person is long gone from the company you’re working. The only way to keep technical decisions without being lost is to document them. But where and how do you do it?

Most often, saving the decisions in some kind of wiki is not going to work. Large documentation files are scarcely updated along with the code, and most developers just lose their focus when they have to move away from the code and filter a large document. What’s more, you can’t keep that kind of files in version control due to their size.

Documenting with Architecture Decision Records.

The key in having successful documentation is to keep it updated in version control in small files. This is what Michael Nygard proposed in his Architecture Decision Records(from now on ADRs). ADRs are text files that save the architecture decisions taken over time and have the following format:

Title: A small descriptive title of the decision.
Context: A description of the constraints under whom the decision was made.
Decision: The actual decision.
Status: Whether or not the decision is proposed/accepted, or amended/superseded by another decision.
Consequences: The consequences that this decision will have.

Keeping ADRs is a powerful technique, as it captures not only the decisions, but under what circumstances were made and documents the consequences at the time that the decision was actually made.

Even when a decision is superseded or amended by another one, Michael Nygard suggests that you never delete it, but mark it as superseded by the new decision. This way you can see how the code has evolved over time and you can also checkout from version control the commit that the decision was made and see what forces drove the author of the decision to make it.

Before presenting the appropriate tooling to successfully keep documentation of our decisions, let’s question ourselves, what kind of decisions should we document? If we end up saving every little detail, then the objective of keeping the documentation as small as possible and to the point fails. As Michael Nygard puts it himself, the decisions that we ought to keep are:

those that affect the structure, non-functional characteristics, dependencies, interfaces, or construction techniques

Tools for using ADRs

So let’s say that you’re now convinced that saving ADRs is a technique worth giving a shot. How should you implement it? Fortunately, Nat Pryce has made available his ADR Tools. ADR Tools is a set of command lines that create ADRs in markdown. The installation instructions are available here. After installing, you just have to type the following command:

adr init your/documentation/directory

This will create the very first ADR, saved in a markdown file. It’s also worth noting that the first thing documented is your decision to start using ADRs.

When you will add a new ADR you will simply give the following command:

adr new Name of the decision

A more exhaustive description of how to use ADR Tools is available here or you can use the command line:

adr help

Except from the help command above, Nat Pryce has also written down the decisions that were made during the development of the Tools as ADRs. This is a helpful example of real life usage of ADRs, that can guide us during our(let’s be honest with ourselves, not so fascinating) journey to meaningful documentation.

Final thoughts

Keeping ADRs seems to be a good way to save decisions that will later give good insight during refactoring. It is quite possible that you will not be the person responsible for that refactoring, or even if you are, then Eagleson’s law will be applied:

Eagleson's Law: Any code of your own that you haven't looked at for six or more months might as well have been written by someone else.

— Programming Wisdom (@codewisdom ) December 10, 2017

Before starting coding a decision that affects the architecture of a system, I think maybe it should be better to write it down as an ADR(just as you would first add a test before the real implementation in TDD).

This way, you are obliged to document the constraints under which you made the decision as the Context of the ADR, and document the Consequences as well, so you are in a position to validate that the change that you are about to introduce is built upon sound foundations.

What’s more, even if you change your mind about that decision and want to amend it with another one, you will already have documented it. The next person that will see the documentation, will be available to see a train of thoughts, and not a ladder that has some steps skipped. The decisions will make more sense, and that person will be in a position to better understand them, and in return accept or modify them.

Branch by Abstraction powered by Kotlin

Kostas Tsalikis — Sun, 21 Jan 2018 18:34:20 +0000

A case of a large-scale refactoring.

Let’s say that one day your senior comes and tells you, “Hi Christine, I’ve checked out this new cool library X that solves problem Y, can you try to change some calls as a proof of concept and in the meantime provide us with your feedback on how it affects us?”

So you become excited, you get the opportunity to dirt your hands with this new hotness that everyone in the community speaks about, and prove yourself to the team. AWESOME!

As you go about the documentation, you get the feeling that this library is really a lifesaver, and should indeed be integrated with the rest of the codebase. On the downside, there is big gotcha. Migrating from the current implementation to the new is not going to be easy. Your only choice is to create a new branch with the new feature, and merge it back to the trunk(a.k.a. master). Or is it not? You don’t want to be the one to make that huge pull request. You only need to implement that small set of simple calls to the new library and leave the rest be. How do you do it?

Enter Branch by Abstraction.

Bearing in mind that you do not want to be in a situation of a huge merge, and that the changes need to be published to the rest of the team as soon as possible, with some research you can stumble upon a technique named “Branch by abstraction”(BbA for the rest of the post). Originally it was introduced by Paul Hammant and described by Jez Humble as well.

But what exactly is BbA and how does it differ from the classic feature branching in version control?

Quoted directly from Martin Fowler:

“Branch by Abstraction” is a technique for making a large-scale change to a software system in gradual way that allows you to release the system regularly while the change is still in-progress.

The name of the technique is a little bit misleading, as it contains the term “branch” in its definition, which almost immediately hints that we’re speaking about branching as in creating a new branch in source control. On the contrary, the technique is advocated by trunk based development, a source control branching model where all developers push directly in the mainline. Rather than branching in source control, the “branching” happens in the code directly.

Let’s say the initial structure of your code is something like this:

Initial Structure.

The steps for BbA are:

Add an abstraction over the current old implementation.
Refactor so all the clients use the abstraction above instead of the old implementation directly.

Steps 1 & 2.

Add the new implementation under that abstraction and gradually delegate to the new implementation as needed.

Step 3.

Once the old implementation is no longer used, it can be deleted.

Step 4.

Once the refactoring is over, delete the abstraction layer.

Step 5.

Although Martin Fowler describes some variations, the general idea is that you create an abstraction over the implementation that needs replacement, find the appropriate behaviour that the abstraction must implement, change the client code to use that abstraction and incrementally add the new code.

What’s more you can use feature toggles, so you continue to deliver software, even if the the new implementation is unfinished.

Pros and Cons

Every technique has its ups and downs, and BbA is no short of its own:

Pros :

No merge hell.
It is possible to extract behaviour that the old implementation should’t have, thus making the system more cohesive.
Code is continuously integrated, thus always on a working state.
You can deliver anytime, even with the feature unfinished.

Cons :

Development process may slow down, as it is not always easy to introduce the required abstraction, or to make the two implementations co-exist during the refactoring.
Difficult to apply if the code needs external audit.

Show me the code

Of course, talk is cheap and a simple but non trivial example is worth a thousand words. Let’s create a simple app which saves quotes and then lists them(named with an extra dose of imagination “QuotesApp”).

For QuotesApp we will use a MVP pattern. The Presenters will hold a QuotesRepository directly instead of UseCases and the QuotesRepository will have a QuotesDataSource for storing the quotes locally. In the role of our legacy library we will use SqlBrite2 and we will try to replace it gradually with the new Room library. The full code of the example is here.

public interface QuotesDataSource {

    Observable<List<Quote>> getSavedQuotes();

    Observable<Boolean> add(Quote quote);

}

public class SqlBriteQuotesDataSource implements QuotesDataSource {

    private final BriteDatabase briteDatabase;

    public SqlBriteQuotesDataSource(BriteDatabase briteDatabase) {
        this.briteDatabase = briteDatabase;
    }

    @Override
    public Observable<List<Quote>> getSavedQuotes() {
        // get quotes with SqlBrite
    }

    @Override
    public Observable<Boolean> add(Quote quote) {
        // save a quote with SqlBrite
    }
}

We will use the QuotesDataSource interface to introduce the abstraction needed for our BbA powered refactoring. In Java this would look something like below. We would make the abstraction implement the QuotesDataSource and then manually delegate all method calls to the old implementation. Then in our dependency injector we would wrap the Sqlite implementation with the new mixed data source.

public class MixedSqliBriteRoomDataSource implements QuotesDataSource {

    private final QuotesDataSource oldDataSource;

    public MixedSqliBriteRoomDataSource(QuotesDataSource oldDataSource) {
        this.oldDataSource = oldDataSource;
    }

    @Override
    public Observable<List<Quote>> getSavedQuotes() {
        return oldDataSource.getSavedQuotes();
    }

    @Override
    public Observable<Boolean> add(Quote quote) {
        return oldDataSource.add(quote);
    }

}

Excuse me, where is my Kotlin twist?

In this example we only have two methods, but one can imagine a huge api with a hundred or more methods. Wouldn’t it be tedious and error prone to manually delegate to the old implementation? So far, Kotlin has not shown up in the post, and now is the time to make its magical appearance. We could use the power of Kotlin’s built in delegation and make the abstraction delegate by default to the old implementation:

class MixedSqliteRoomDataSource(private val oldDataSource: QuotesDataSource)
: QuotesDataSource by oldDataSource

This is not only terse and concise, but also saves you from the errors that anyone can easily do during manual delegation.

Back to Refactoring

Continuing with the refactoring, we would introduce the new implementation, and pass it as a second argument to our abstraction. Then finally, we would override only some of the methods in our abstraction to delegate to the new implementation.

class MixedSqliteRoomDataSource(private val oldDataSource: QuotesDataSource,
                                private val newDataSource: QuotesDataSource)
    : QuotesDataSource by oldDataSource {

    override fun getSavedQuotes(): Observable<MutableList<Quote>> {
        return newDataSource.savedQuotes
    }

}

This is much easier to push directly to the mainline, or if you’re in favour of code reviews, make a short lived version control branch and open a pull request from there.

As you add more and more methods in your new implementation, the old one and the abstraction will become obsolete, and finally you will be able to delete them.

Final Thoughts

So far, BbA seems to be quite a sane way for making large-scale changes in your codebase. It may not be always easy to make the old and new implementations coexist, but all in all, it seems to be worth the effort. What’s more, we can use the power of Kotlin’s delegation to quickly implement only a set of behaviours in order to provide a proof of concept, and then continue with the rest of the refactoring.

Blogging with Hugo and Netlify

Kostas Tsalikis — Sat, 13 Jan 2018 22:34:25 +0000

Hi everyone, and welcome to my first ever blogpost! I’ve been meaning to begin a technical blog about Android development for ages, and at last the time has come. This first post, however, will not be Android related – instead I will break down why and how I chose Hugo and deployed this site.

Hugo vs Jekyll

As I needed something easy to setup and customize, a static site made perfect sense – and there are lots of static site generators to choose from. In fact there are too many, so I decided to play with the two highest ranked, Jekyll (Ruby) and Hugo (Go), which both have strong communities and offer a plethora of themes.

Both are fun, but with Jekyll you have to deal with Ruby and RubyGems. On the other hand, the only thing that Hugo needs is the Go executable. In the end I chose Hugo, as I wouldn’t have to live with the pain of dealing with Ruby again in case of migrating to another environment, and overall I felt that Hugo worked pretty straightforward for me. Installing and tweaking Hugo is pretty easy if you follow the documentation, so I’ve left it outside of the scope of this post.

Hosting on Netlify

As an Android developer, I cannot say that I had prior experience on how to deploy a site. What I can say, however, is that this was one of the biggest hurdles that I faced in the process of setting up this blog. Fortunately, Hugo has a lot of suggestions on hosting and deployment which saved the day. Among the proposals, that of Netlify seemed to be the most attractive, with the promise of Continuous Deployment, custom domain name and 1-click HTTPS.

Of course, there was no way that the whole setup would go as planned right away. Firstly, I wasn’t even able to deploy the site. Being quite forgetful myself, I had missed to add the wonderful Hugo theme that I had chosen as a git submodule. After that, the site was up, but something in the theming had gone awfully wrong. Once again, I had forgotten to change the baseURL in the config.toml, which contains configuration for the Hugo site.

And then, oh joy! The site was live – but not exactly as I had managed to do locally in my machine. Since everything was a draft, the blog was just empty. The trick was to add a netlify.toml file, which configures the Netlify deployment, and edit it to run a different command that builds drafts in Hugo. Then with a pull request in Github, a new preview environment was created in Netlify, and I could verify that the blog was up as intended with the drafts built. This is extremely useful, because we can share drafts with reviewers or make UI changes without affecting the production site.

Ready, aim, fire!

Finally, with the site live, I’m ready to start my journey in technical blogging. I wish to be a frequent writer and share my ideas (mostly) about Android development with the rest of the community. Hopefully, this blog will be a platform where knowledge will be shared among us, both for me and for you – the readers – in an environment of mutual respect and understanding.