I See CI

From Droidcon Dubai 2018 by Ahmed El-Helw and Oubai Abbasi

Investment in your CI infrastructure pays back many fold because it makes all members of your team more productive and increases code quality. In this talk, we’ll go through some of the things Careem does for our CI

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. What we do at Careem
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. 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 } }
73. What do we do with dumped requests?
74. We save them as a Postman Collection
75. We save them as a Postman Collection • Nice GUI • Easy to edit responses and search requests • Supports placeholders for path, headers or body
76. Now what?
77. @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()); }
78. @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()); }
79. @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()); }
80. fun mount(collectionName: String, fileName: String) { 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. 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") }
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. 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) } } })
87. How to confirm the UI looks right?
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. @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); }
90. 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
91. Making it cross platform • Share API data • Run a python mock server instead of OkHttp • Share screenshot diff-ing logic/UI
92. Hidden Changes
93. Photo from LifeWithCats
94. Permissions
95. 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'
96. dependencies { implementation deps.instabug.instabug // more dependencies }
97. aapt d permissions ./app/build/outputs/apk/debug/app-debug.apk
98. › 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'
99. › 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
100. Method Count
101. > 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
103. dependencies { implementation deps.instabug.instabug // more dependencies }
104. > 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
105. Delta methods: +17129 Delta fields: +12385 Delta classes: +2329
106. APK Size
107. 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
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 :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({ message: "new version uploaded to S3", channel: “#qa" }) end
112. lane :alpha do gradle(task: "clean assembleDebug test android") aws_s3 slack({ message: "new version uploaded to S3", channel: “#qa" }) end
113. lane :release do gradle(task: "clean assembleRelease test android") supply slack({ message: "new version released", channel: “#announcements" }) end
114. lane :release do gradle(task: "clean assembleRelease test android") supply slack({ message: "new version released", channel: “#announcements" }) end
115. Code Coverage
120. Photo by Robert Zunikoff on Unsplash
121. Photo by Annie Spratt on Unsplash Where?
122. Compilation time Pull request time Commit time Post-commit time Photo by Ales Krivec on Unsplash
123. Photo by Evan Kirby on Unsplash Gotchas • Flaky Tests • Tool version updates • Unstable environment • Crashing tests
124. Test Orchestrator
125. • Invest in improving build times • Set up static analysis tools • Optimize for code quality and stability Summary
127. Photo by Bryan Minear on Unsplash
128. 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/