I See Continuous Improvement

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

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
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]
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
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
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
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
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
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