Allow configuring Gradle, AGP, various SDK and Kotlin versions for Espresso server #496
Conversation
|
|
|
We will need the feature and all the properties documented properly. https://github.com/appium/appium/tree/master/docs/en/writing-running-appium/android would be a good place to keep the document. Altogether, I like the implementation, good job. |
|
Awesome! And it's good to add a description about the caps in https://github.com/appium/appium/blob/master/docs/en/writing-running-appium/caps.md after finalising this change. |
dpgraham
changed the title
|
I've done some additional testing and found that our app require additional dependency on What do you think? |
|
ah, nice. thanks :) (I think only adding them is enough so far.) |
|
@mykola-mokhnach @tinder-ktarasov let me know if I can help testing, at least :) |
|
It is necessary to make the tests green though. |
|
@mykola-mokhnach it doesn't look like the failing test case can be affected by these changes. Could you confirm that the test case about pointers is passing on |
|
@hunzai you could check if |
|
@tinder-ktarasov Yep, the test is flaky. Restarting the job usually helps to resolve the problem |
lib/server-builder.js
Outdated
| // if we have an error we want to output the logs | ||
| // otherwise the failure is inscrutible | ||
| // but do not log expected errors | ||
| const ignoreError = IGNORED_ERRORS.some((x) => line.includes(x)); |
There was a problem hiding this comment.
Why do we keep IGNORED_ERRORS array empty?
There was a problem hiding this comment.
For possible future filtering. Removed it now.
lib/server-builder.js
Outdated
| const ignoreError = IGNORED_ERRORS.some((x) => line.includes(x)); | ||
|
|
||
| if (this.showGradleLog && !ignoreError) { | ||
| gradleLog.error(line); |
There was a problem hiding this comment.
are all strings in the Gradle log errors? I would rather print them at debug or info level
There was a problem hiding this comment.
Probably keeping STDERR lines as error (that would be actual errors and task names) and changing other to info would be better?
There was a problem hiding this comment.
error is still too much. We only mark critical things as error. I would mark them as warnings and normal messages as info
|
@tinder-ktarasov It would be nice to document the feature properly. You could put the resulting document to https://github.com/appium/appium/tree/master/docs/en/writing-running-appium/android |
|
👍 |
|
@tinder-ktarasov do you want an invite to the Appium Slack channel? |
|
@mykola-mokhnach @KazuCocoa @dpgraham, sorry for being absent. I've added docs about this feature - appium/appium#13766 |
Currently, appium-espresso-driver uses same Gradle, AGP, SDK, and Kotlin versions when building espresso-server for all apps. Such versions might be different from the versions used to build AUT. Such setup is unusual as androidTest APKs are usually built with the same SDKs as the main app. It can lead to various issues as Android building infrastructure is quite complex.
This PR proposes changes that will allow configuring those versions to keep them in sync with the versions in the main app.
To build espresso-server with custom versions forceEspressoRebuild capability should be set to true.
I didn't change anything regarding caching, so I suppose rebuilt APK is not cached and it will be rebuilt even when versions are same.
Unfortunately, it doesn't fix that Resource issue, I'll make a separate PR for it.New desired capabilities
Build configuration JSON keys
Configuration example:
{ "toolsVersions": { "gradle": "5.1.1", "androidGradlePlugin": "3.4.2", "compileSdk": 28, "buildTools": "28.0.3", "minSdk": 18, "targetSdk": 28, "kotlin": "1.3.31" }, "additionalAppDependencies": [ "com.google.android.material:material:1.0.0" ] }Minimum supported versions here and default versions are current versions in espresso-server project (not changed in this PR)
Possible improvements (not implemented)