I See Continuous Improvement

I See Continuous Improvement

How setting up CI for your Android projects leads to continuous improvement of the codebase.

F60e42d94f99f029b590206076dbd354?s=128

Ahmed El-Helw

April 13, 2018
Tweet

Transcript

  1. I see Continuous Improvement Oubai Abbasi and Ahmed El-Helw

  2. How do we know the stability of a given build?

  3. How do we maintain code quality as the team grows?

  4. How do we quickly find bugs before they make their

    way into production?
  5. How do we make builds available more widely?

  6. How do we uncover hidden changes in our apps?

  7. Why? • Maintain a stylistically consistent code base • Catch

    common bugs and design flaws early on • Encourage a test driven culture • Build trust in the code quality
  8. None
  9. Static Analysis Tools Catching issues early

  10. “Checkstyle is a development tool to help programmers write Java

    code that adheres to a coding standard.”
  11. “It automates the process of checking Java code to spare

    humans of this boring (but important) task. This makes it ideal for projects that want to enforce a coding standard.”
  12. package ae.droidcon.checkstyle; class Example { static int add(int first, int

    second) { return first + second; } }
  13. [ant:checkstyle] [WARN] ~/code/experimentation/src/ main/java/ae/droidcon/checkstyle/Example.java:5:1: Line contains a tab character. [FileTabCharacter]

  14. None
  15. package ae.droidcon.checkstyle; public class Example< T> { }

  16. [ant:checkstyle] [WARN] ~/code/experimentation/src/ main/java/ae/droidcon/checkstyle/Example.java:3:32: GenericWhitespace '<' is followed by whitespace.

    [GenericWhitespace]
  17. package ae.droidcon.checkstyle; class Example { static void awesome (int a,

    int b) { System.out.println(a + ", " + b); } }
  18. [ant:checkstyle] [WARN] ~/code/experimentation/src/ main/java/ae/droidcon/checkstyle/Example.java:4:23: '(' is preceded with whitespace. [MethodParamPad]

  19. • Avoid import java.util.*; • Import order • Modifier order

    • Unnecessary parentheses Other Stylistic Issues
  20. “It can find class design problems, method design problems.”

  21. class Example { int useless(int value) { value *= 2;

    return value; } }
  22. [ant:checkstyle] [WARN] ~/code/experimentation/src/ main/java/ae/droidcon/checkstyle/Example.java:5:11: Assignment of parameter 'value' is not

    allowed. [ParameterAssignment]
  23. package ae.droidcon.checkstyle; class Example { void play() { int fantasy

    = 7; int offset = 0; System.out.println(fantasy + offset); offset += 3; System.out.println(fantasy + offset); } }
  24. [ant:checkstyle] [WARN] ~/code/experimentation/src/ main/java/ae/droidcon/checkstyle/Example.java:5:9: Variable 'fantasy' should be declared final.

    [FinalLocalVariable]
  25. package ae.droidcon.checkstyle; class Util { static void op() {} static

    void secondOp() {} }
  26. [ant:checkstyle] [WARN] ~/code/experimentation/src/ main/java/ae/droidcon/checkstyle/Util.java:3:1: Utility classes should not have a

    public or default constructor. [HideUtilityClassConstructor]
  27. Lint Photo by Andrew Neel on Unsplash

  28. object Caller { fun whoYouGonnaCall(context: Context) { val intent =

    Intent(Intent.ACTION_CALL, Uri.parse("tel:" + "GHOSTBUSTERS")) context.startActivity(intent) } }
  29. ~/code/experiments/app/src/main/kotlin/ae/droidcon/ lint/Caller.kt:12: Error: Missing permissions required by intent Intent.ACTION_CALL: android.permission.CALL_PHONE

    [MissingPermission] context.startActivity(intent)
  30. class BoringActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) {

    super.onCreate(savedInstanceState) setContentView(R.layout.demo) val button: ImageButton = findViewById(R.id.button) button.setOnClickListener { } } }
  31. ~/code/experiments/app/src/main/java/ae/droidcon/lint/ BoringActivity.kt:14: Error: Unexpected implicit cast to ImageButton: layout tag

    was Button [WrongViewCast] val button: ImageButton = findViewById(R.id.button)
  32. override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.demo) val button: Button

    = findViewById(R.id.button) button.setOnClickListener { } val drawable = ColorDrawable( R.color.accent_material_dark) button.background = drawable }
  33. ~/code/experiments/app/src/main/java/ae/droidcon/ lint/BoringActivity.kt:20: Error: Call requires API level 16 (current min

    is 14): android.view.View#setBackground [NewApi] button.background = drawable
  34. ~/code/experiments/app/src/main/java/ae/droidcon/ lint/BoringActivity.kt:19: Error: Should pass resolved color instead of resource

    id here: getResources().getColor(R.color.accent_material_dark) [ResourceAsColor] R.color.accent_material_dark)
  35. • WrongThread • LongLogTag • SQLiteString • … and many

    more Additional Checks
  36. None
  37. Error Prone

  38. “Using Error Prone to augment the compiler’s type analysis, you

    can catch more mistakes before they cost you time, or end up as bugs in production.”
  39. public Collection<String> sort(Collection<String> foos) { Collections.sort(new ArrayList<>(foos)); return foos; }

  40. ~/code/experimentation/src/main/java/ae/droidcon/ errorprone/Util.java:9: error: [UnusedCollectionModifiedInPlace] Collection is modified in place, but

    the result is not used Collections.sort(new ArrayList<>(foos)); ^ (see http://errorprone.info/bugpattern/ UnusedCollectionModifiedInPlace)
  41. public class Carbon { public SimpleDateFormat dating() { return new

    SimpleDateFormat("YYYY-MM-dd"); } }
  42. ~/code/experimentation/src/main/java/ae/droidcon/ errorprone/Carb.java:7: error: [MisusedWeekYear] Use of "YYYY" (week year) in

    a date pattern without "ww" (week in year). You probably meant to use "yyyy" (year) instead. return new SimpleDateFormat("YYYY-MM-dd"); ^ (see http://errorprone.info/bugpattern/ MisusedWeekYear) Did you mean 'return new SimpleDateFormat("yyyy- MM-dd");'?
  43. public class Finder { public boolean findWaldo(Set<Long> values) { return

    (values.contains(1)); } }
  44. ~/code/experimentation/src/main/java/ae/droidcon/ errorprone/Finder.java:7: error: [CollectionIncompatibleType] Argument '1' should not be passed

    to this method; its type int is not compatible with its collection's type argument Long return (values.contains(1)); ^ (see http://errorprone.info/bugpattern/ CollectionIncompatibleType)
  45. public class PostApocolypse { private final Set<Human> theLastOfUs = World.getRemainingHumans();

    private volatile int zombies = 9001; public void fightZombies() { zombies--; } }
  46. ~/code/experimentation/src/main/java/ae/droidcon/ errorprone/PostApocolypse.java:9: warning: [NonAtomicVolatileUpdate] This update of a volatile variable

    is non-atomic zombies--; ^ (see http://errorprone.info/bugpattern/ NonAtomicVolatileUpdate) 1 warning
  47. public String getFaction(int version) { String result; switch (version) {

    case FALLOUT_2: case FALLOUT_NEW_VEGAS: result = "New California Rangers"; break; case FALLOUT_3: result = "The Enclave"; break; case FALLOUT_4: result = "Institute"; default: result = "War never changes"; } return result; }
  48. ~/code/experimentation/src/main/java/ae/droidcon/ errorprone/Fallout.java:17: warning: [FallThrough] Execution may fall through from the

    previous case; add a `// fall through` comment before this line if it was deliberate default: ^ (see http://errorprone.info/bugpattern/ FallThrough) 1 warning
  49. public class TemplarBase { protected Assassin ezio; public Creed getCreed()

    { return ezio.getCreed(); } } public class TemplarCastle extends TemplarBase { protected Assassin ezio; }
  50. public class TemplarBase { protected Assassin ezio; public Creed getCreed()

    { return ezio.getCreed(); } } public class TemplarCastle extends TemplarBase { protected Assassin ezio; }
  51. ~/code/experimentation/src/main/java/ae/droidcon/ errorprone/TemplarCastle.java:4: warning: [HidingField] Hiding fields of superclasses may cause

    confusion and errors. This field is hiding a field of the same name in superclass: TemplarBase protected Assassin ezio; ^ (see http://errorprone.info/bugpattern/ HidingField) 1 warning
  52. • MissingOverride • ClassCanBeStatic • RemoveUnusedImports • SelfAssignment Additional Checks

  53. • InsecureCryptoUsage • FragmentInjection • UseBinds Security/Perf Checks

  54. Infer

  55. class Persona { private final Mementos mementos = new Mementos();

    Shadow getShadowForPerson(String person) { if (mementos.hasShadowFor(person)) { return new Shadow(person); } return null; } void destroyShadow(String person) { final Shadow shadow = getShadowForPerson(person); shadow.unshadow(); } }
  56. class Persona { private final Mementos mementos = new Mementos();

    Shadow getShadowForPerson(String person) { if (mementos.hasShadowFor(person)) { return new Shadow(person); } return null; } void destroyShadow(String person) { final Shadow shadow = getShadowForPerson(person); shadow.unshadow(); } }
  57. Found 1 issue Persona.java:13: error: NULL_DEREFERENCE object `shadow` last assigned

    on line 12 could be null and is dereferenced at line 13. 11. void destroyShadow(String person) { 12. final Shadow shadow = getShadowForPerson(person); 13. > shadow.unshadow(); 14. } 15. }
  58. • Context leaks • Resource leaks • Unsafe @GuardedBy access

    Additional Checks
  59. None
  60. Testing

  61. How to UI test with network calls

  62. Hitting the real APIs • Unreliable • Can’t test edge

    cases • How to test payment related scenarios
  63. Hitting the real APIs • Unreliable • Can’t test edge

    cases • How to test payment related scenarios
  64. Building a mock interface •Reliable •Requires more coding, maintenance and

    testing
  65. Building a mock interface •Reliable •Requires more coding, maintenance and

    testing
  66. Mock web server •No extra code •Still hard to maintain

    and prepare
  67. class RecordCallsInterceptor constructor(context: Context, val gson: Gson) : Interceptor {

    ... override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val response = chain.proceed(request) ... val recordModel = ApiRecordModel( request = ApiRecordRequestModel( path = request.url().encodedPath(), method = request.method(), headers = requestHeaders, body = requestBody ), response = ApiRecordResponseModel( code = response.code(), headers = responseHeaders, body = responseBody ) ) ... dumpToServer(recordModel) return response } }
  68. class RecordCallsInterceptor constructor(context: Context, val gson: Gson) : Interceptor {

    ... override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val response = chain.proceed(request) ... val recordModel = ApiRecordModel( request = ApiRecordRequestModel( path = request.url().encodedPath(), method = request.method(), headers = requestHeaders, body = requestBody ), response = ApiRecordResponseModel( code = response.code(), headers = responseHeaders, body = responseBody ) ) ... dumpToServer(recordModel) return response } }
  69. class RecordCallsInterceptor constructor(context: Context, val gson: Gson) : Interceptor {

    ... override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val response = chain.proceed(request) ... val recordModel = ApiRecordModel( request = ApiRecordRequestModel( path = request.url().encodedPath(), method = request.method(), headers = requestHeaders, body = requestBody ), response = ApiRecordResponseModel( code = response.code(), headers = responseHeaders, body = responseBody ) ) ... dumpToServer(recordModel) return response } }
  70. class RecordCallsInterceptor constructor(context: Context, val gson: Gson) : Interceptor {

    ... override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val response = chain.proceed(request) ... val recordModel = ApiRecordModel( request = ApiRecordRequestModel( path = request.url().encodedPath(), method = request.method(), headers = requestHeaders, body = requestBody ), response = ApiRecordResponseModel( code = response.code(), headers = responseHeaders, body = responseBody ) ) ... dumpToServer(recordModel) return response } }
  71. class RecordCallsInterceptor constructor(context: Context, val gson: Gson) : Interceptor {

    ... override fun intercept(chain: Interceptor.Chain): Response { val request = chain.request() val response = chain.proceed(request) ... val recordModel = ApiRecordModel( request = ApiRecordRequestModel( path = request.url().encodedPath(), method = request.method(), headers = requestHeaders, body = requestBody ), response = ApiRecordResponseModel( code = response.code(), headers = responseHeaders, body = responseBody ) ) ... dumpToServer(recordModel) return response } }
  72. What do we do with dumped requests?

  73. We save them as a Postman Collection

  74. We save them as a Postman Collection • Nice GUI

    • Easy to edit responses and search requests • Supports placeholders for path, headers or body
  75. Now what?

  76. @Test public void loginTest_success() { mount("defaultCollection"); onView(withId(R.id.email)).perform(typeText("test@email.com")); onView(withId(R.id.edt_password)).perform(typeText("Password"), closeSoftKeyboard()); onView(withId(R.id.btn_login_with_fb)).perform(click());

    }
  77. @Test public void loginTest_success() { mount("defaultCollection"); onView(withId(R.id.email)).perform(typeText("test@email.com")); onView(withId(R.id.password)).perform(typeText("Password"), closeSoftKeyboard()); onView(withId(R.id.btn_login)).perform(click());

    }
  78. @Test public void loginTest_success() { mount("defaultCollection"); onView(withId(R.id.email)).perform(typeText("test@email.com")); onView(withId(R.id.edt_password)).perform(typeText("Password"), closeSoftKeyboard()); onView(withId(R.id.btn_login_with_fb)).perform(click());

    }
  79. fun mount(collectionName: String, fileName: String) { cachedCollections[collectionName] = file.item.firstOrNull {

    collection -> collection.name==collectionName }?: throw IllegalArgumentException("Collection not found: $collectionName") }
  80. fun mount(collectionName: String, fileName: String) { val file = PostmanLoader.load(fileName)

    cachedCollections[collectionName] = file.item.firstOrNull { collection -> collection.name == collectionName }?: throw IllegalArgumentException("Collection not found: $collectionName") }
  81. fun mount(collectionName: String, fileName: String) { val file = PostmanLoader.load(fileName)

    cachedCollections[collectionName] = file.item.firstOrNull { collection -> collection.name == collectionName }?: throw IllegalArgumentException("Collection not found: $collectionName") }
  82. MockWebServer().apply { setDispatcher(object : Dispatcher() { override fun dispatch(request: RecordedRequest):

    MockResponse { return mountedCollections .firstOrNull { requestsMatch(request, it.request) }?.let { createResponseFromPostman(it.response[0]) } ?: MockResponse().apply { setResponseCode(404) } } })
  83. MockWebServer().apply { setDispatcher(object : Dispatcher() { override fun dispatch(request: RecordedRequest):

    MockResponse { return mountedCollections .firstOrNull { requestsMatch(request, it.request) }?.let { createResponseFromPostman(it.response[0]) } ?: MockResponse().apply { setResponseCode(404) } } })
  84. MockWebServer().apply { setDispatcher(object : Dispatcher() { override fun dispatch(request: RecordedRequest):

    MockResponse { return mountedCollections .firstOrNull { requestsMatch(request, it.request) }?.let { createResponseFromPostman(it.response[0]) } ?: MockResponse().apply { setResponseCode(404) } } })
  85. MockWebServer().apply { setDispatcher(object : Dispatcher() { override fun dispatch(request: RecordedRequest):

    MockResponse { return mountedCollections .firstOrNull { requestsMatch(request, it.request) }?.let { createResponseFromPostman(it.response[0]) } ?: MockResponse().apply { setResponseCode(404) } } })
  86. How to confirm the UI looks right?

  87. @Test public void loginTest_success() { mount("defaultCollection"); screenshot("loginTest", "success", "emptyFields", lang);

    onView(withId(R.id.email)).perform(typeText("test@email.com")); onView(withId(R.id.edt_password)).perform(typeText("Password"), closeSoftKeyboard()); screenshot("loginTest", "success", "filledFields", lang); onView(withId(R.id.btn_login_with_fb)).perform(click()); screenshot("loginTest", "success", "loading", lang); }
  88. @Test public void loginTest_success() { mount("defaultCollection"); screenshot("loginTest", "success", "emptyFields", lang);

    onView(withId(R.id.email)).perform(typeText("test@email.com")); onView(withId(R.id.edt_password)).perform(typeText("Password"), closeSoftKeyboard()); screenshot("loginTest", "success", "filledFields", lang); onView(withId(R.id.btn_login_with_fb)).perform(click()); screenshot("loginTest", "success", "loading", lang); }
  89. What to do with screenshot? • Upload them to your

    server • Tag them with configuration, test, scenario, date • Run image diffing (ie: http://www.imagemagick.org/ Usage/compare/) • Flag different/new screenshots for manual verification
  90. Making it cross platform • Share API data • Run

    a python mock server instead of OkHttp • Share screenshot diff-ing logic/UI
  91. Hidden Changes

  92. Photo from LifeWithCats

  93. Permissions

  94. aapt d permissions ./app/build/outputs/apk/debug/app-debug.apk uses-permission: name='android.permission.INTERNET' uses-permission: name='android.permission.WRITE_EXTERNAL_STORAGE' uses-permission: name='android.permission.ACCESS_NETWORK_STATE'

    uses-permission: name='android.permission.WAKE_LOCK' uses-permission: name='android.permission.READ_EXTERNAL_STORAGE'
  95. dependencies { implementation deps.helpbug.helpbug // more dependencies }

  96. aapt d permissions ./app/build/outputs/apk/debug/app-debug.apk

  97. › diff before after 6a7,9 > uses-permission: name='android.permission.ACCESS_WIFI_STATE' > uses-permission:

    name='android.permission.RECORD_AUDIO' > uses-permission: name='android.permission.MODIFY_AUDIO_SETTINGS' Picture from principedifino
  98. Method Count

  99. > Task :app:countDebugDexMethods Total methods in app-debug.apk: 52673 (80.37% used)

    Total fields in app-debug.apk: 30053 (45.86% used) Total classes in app-debug.apk: 6962 (10.62% used) Methods remaining in app-debug.apk: 12862 Fields remaining in app-debug.apk: 35482 Classes remaining in app-debug.apk: 58573
  100. None
  101. dependencies { implementation deps.helpbug.helpbug // more dependencies }

  102. > Task :app:countDebugDexMethods Total methods in app-debug.apk: 69802 (106.51% used)

    Total fields in app-debug.apk: 42438 (64.76% used) Total classes in app-debug.apk: 9291 (14.18% used) Methods remaining in app-debug.apk: 0 Fields remaining in app-debug.apk: 23097 Classes remaining in app-debug.apk: 56244
  103. Delta methods: +17129 Delta fields: +12385 Delta classes: +2329

  104. APK Size

  105. What else can we do? •Upload apk to S3 bucket

    for internal distribution •Automatically upload builds to Google Play •Post on slack about new builds
  106. lane :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({

    message: "new version uploaded to S3", channel: “#qa" }) end
  107. lane :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({

    message: "new version uploaded to S3", channel: “#qa" }) end
  108. lane :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({

    message: "new version uploaded to S3", channel: “#qa" }) end
  109. lane :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({

    message: "new version uploaded to S3", channel: “#qa" }) end
  110. lane :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({

    message: "new version uploaded to S3", channel: “#qa" }) end
  111. lane :release do gradle(task: "clean assembleRelease test android") supply slack({

    message: "new version released", channel: “#announcements" }) end
  112. lane :release do gradle(task: "clean assembleRelease test android") supply slack({

    message: "new version released", channel: “#announcements" }) end
  113. Code Coverage

  114. None
  115. None
  116. None
  117. None
  118. Photo by Robert Zunikoff on Unsplash

  119. Photo by Annie Spratt on Unsplash Where?

  120. Compilation time Pull request time Commit time Post-commit time Photo

    by Ales Krivec on Unsplash
  121. Photo by Evan Kirby on Unsplash Gotchas • Flaky Tests

    • Tool version updates • Unstable environment • Crashing tests
  122. Test Orchestrator

  123. • Invest in improving build times • Set up static

    analysis tools • Optimize for code quality and stability Summary
  124. None
  125. Photo by Bryan Minear on Unsplash

  126. References • https://speakerdeck.com/kaskasi/i-am-on-a-fastlane-to-hell • https://medium.com/square-corner-blog/surfacing-hidden- change-to-pull-requests-6a371266e479 • https://github.com/KeepSafe/dexcount-gradle-plugin • https://github.com/JakeWharton/dex-method-list

    • http://www.imagemagick.org/Usage/compare/ • https://android.github.io/kotlin-guides/interop.html