Add test apps template and fireci commands to measure sdk startup times. #2611
Conversation
Coverage ReportAffected SDKs
Test Logs
NotesHTML coverage reports can be produced locally with Head commit (01dd79eb) is created by Prow via merging commits: 3f0779e 1d71481. |
Binary Size ReportAffected SDKsNo changes between base commit (3f0779e) and head commit (01dd79eb). Test Logs
NotesHead commit (01dd79eb) is created by Prow via merging commits: 3f0779e 1d71481. |
There was a problem hiding this comment.
My understanding was that mactobenchmark would contain:
- config.yaml
- template/ dir
However it contains many files that look like the could be generated, like the base gradle build as well as gradle wrapper itself(can we use gradle wrapper from the root directory)?
| gradle.run('assembleAllForSmokeTests') | ||
| artifact_versions = _parse_artifacts() | ||
|
|
||
| os.chdir('macrobenchmark') |
There was a problem hiding this comment.
Would it make sense to use the contextmanager you have below? i.e. with chdir('foo')
There was a problem hiding this comment.
This file is now renamed to macrobenchmark.py. This method call is replaced with with chdir('macrobenchmark'):, at line 48.
|
I have updated this pull request with some changes:
Can you help take another look at this pull request? Thanks! |
|
|
||
| async def _parse_artifact_versions(): | ||
| proc = await asyncio.subprocess.create_subprocess_exec('./gradlew', 'assembleAllForSmokeTests') | ||
| await proc.wait() |
There was a problem hiding this comment.
wouldn't line 56 await be enough? not sure
There was a problem hiding this comment.
Sounds to me it is required. So if I don't await on it, it gives me this warning:
/usr/local/Cellar/python/3.7.7/Frameworks/Python.framework/Versions/3.7/lib/python3.7/asyncio/unix_events.py:878: RuntimeWarning: A loop is being detached from a child watcher with pending handlers
RuntimeWarning)
My understanding is that it means the underneath Future object may not be settled before my code exit. Also in the python doc for asyncio subprocess, the first example and the last example also await on proc.communicate() and proc.wait() respectively.
This also reminds me that I forgot to await at other places in the code. The problem was masked previously. I fixed in the new commit.
| proc = await asyncio.subprocess.create_subprocess_exec('./gradlew', 'assembleAllForSmokeTests') | ||
| await proc.wait() | ||
|
|
||
| with open('build/m2repository/changed-artifacts.json', 'r') as json_file: |
|
|
||
| async def _create_gradle_wrapper(): | ||
| with open('macrobenchmark/settings.gradle', 'w'): | ||
| pass |
There was a problem hiding this comment.
Yes. Without a settings.gradle file, ./gradlew wrapper failed with this error:
FAILURE: Build failed with an exception.
* What went wrong:
Project directory '/Users/yifany/Documents/firebase/firebase-android-sdk/macrobenchmark' is not part of the build defined by settings file '/Users/yifany/Documents/firebase/firebase-android-sdk/settings.gradle'. If this is an unrelated build, it must have its own settings file.
And looks like an empty settings.gradle is good enough.
| asyncio.run(_launch_macrobenchmark_test()) | ||
|
|
||
|
|
||
| async def _launch_macrobenchmark_test(): |
There was a problem hiding this comment.
What is the motivation for all the asyncs? not sure it's warranted for this simple use case
There was a problem hiding this comment.
Here is how I come to all these async code:
- gcloud calls takes long time, and I want to parallelize this part
- now that test apps for different SDKs are isolated, to build them I need to run
./gradlew assemble assembleAndroidTestone by one in each rootprojects for all SDKs (previously all test apps are subprojects of the same root project, so I only had to run one gradle command, andorg.gradle.parallel=truetakes care of the parallelization). Anyway I think I want to parallelize this part as well - with Popen and blocking wait, I'm able to make apk assembling and FTL uploading for different SDKs concurrent, respectively. But with async code, it's convenient for me to even make uploading for one SDK only depend on its corresponding assembling.
- It also seems easier for me to handle stdout/stderr streaming (with color and sdk name decoration) to parent process from multiple subprocesses with async.
- at this point, I feel like it's actually easier for me to write async code.
I do agree that it brings a lot of (unnecessarily) complexity. Maybe there are simpler ways to do it without async, and I just have not found them. What do you think? I'd like to have a quick GVC if you have time?
There was a problem hiding this comment.
Actually, I take it back. It does seem to provide a lot of value, not the least of which is that it provides concurrency out of the box without having to deal with it explicitly and makes the code to read simple. Using asyncio sgtm
|
@yifanyang: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
No description provided.