Improve coverage report format in GitHub pull requests. #1377

yifanyang · 2020-03-20T16:33:24Z

Calculate and upload coverage reports to our own Metrics Service instead
of Codecov.

google-oss-bot · 2020-03-20T16:44:03Z

Binary Size Report

Affected SDKs

SDK	Type	Base (`b878917`)	Head (`aa1370d`)	Diff
firebase-inappmessaging	apk (release)	?	3256643.00	? (?)
	aar	?	467661.00	? (?)
	apk (aggressive)	?	601340.00	? (?)
firebase-common:ktx	aar	?	5965.00	? (?)
protolite-well-known-types	apk (release)	?	561089.00	? (?)
	aar	?	1203203.00	? (?)
	apk (aggressive)	?	122384.00	? (?)
firebase-inappmessaging:ktx	aar	?	5003.00	? (?)
firebase-segmentation	apk (release)	?	1667839.00	? (?)
	aar	?	35427.00	? (?)
	apk (aggressive)	?	1017130.00	? (?)
firebase-database:ktx	aar	?	6706.00	? (?)
firebase-functions:ktx	aar	?	5844.00	? (?)
firebase-storage	apk (release)	?	976604.00	? (?)
	aar	?	119257.00	? (?)
	apk (aggressive)	?	325641.00	? (?)
firebase-common	apk (release)	?	646638.00	? (?)
	aar	?	34517.00	? (?)
	apk (aggressive)	?	82939.00	? (?)
encoders:firebase-encoders-json	aar	?	15335.00	? (?)
firebase-firestore:ktx	aar	?	7093.00	? (?)
firebase-crashlytics-ndk	apk (release)	?	1936966.00	? (?)
	aar	?	598746.00	? (?)
	apk (aggressive)	?	1170706.00	? (?)
transport:transport-api	aar	?	6439.00	? (?)
transport:transport-backend-cct	aar	?	38343.00	? (?)
firebase-inappmessaging-display:ktx	aar	?	22190.00	? (?)
firebase-database	apk (release)	?	1101570.00	? (?)
	aar	?	480458.00	? (?)
	apk (aggressive)	?	325607.00	? (?)
encoders:firebase-encoders-reflective	aar	?	7650.00	? (?)
firebase-crashlytics	apk (release)	?	1353912.00	? (?)
	aar	?	400122.00	? (?)
	apk (aggressive)	?	584013.00	? (?)
transport:transport-runtime	aar	?	122725.00	? (?)
firebase-installations	apk (release)	?	665523.00	? (?)
	aar	?	55057.00	? (?)
	apk (aggressive)	?	84604.00	? (?)
firebase-config:ktx	aar	?	6162.00	? (?)
firebase-dynamic-links	apk (release)	?	951227.00	? (?)
	aar	?	51149.00	? (?)
	apk (aggressive)	?	327459.00	? (?)
firebase-storage:ktx	aar	?	6143.00	? (?)
firebase-installations-interop	apk (release)	?	616109.00	? (?)
	aar	?	7509.00	? (?)
	apk (aggressive)	?	61699.00	? (?)
firebase-components	apk (release)	?	25749.00	? (?)
	aar	?	34495.00	? (?)
	apk (aggressive)	?	10959.00	? (?)
firebase-abt	apk (release)	?	746406.00	? (?)
	aar	?	35383.00	? (?)
	apk (aggressive)	?	85717.00	? (?)
firebase-config	apk (release)	?	1143995.00	? (?)
	aar	?	214548.00	? (?)
	apk (aggressive)	?	395817.00	? (?)
firebase-datatransport	apk (release)	?	711393.00	? (?)
	aar	?	5041.00	? (?)
	apk (aggressive)	?	116347.00	? (?)
firebase-dynamic-links:ktx	aar	?	7877.00	? (?)
firebase-inappmessaging-display	apk (release)	?	4520175.00	? (?)
	aar	?	165930.00	? (?)
	apk (aggressive)	?	1603058.00	? (?)
firebase-functions	apk (release)	?	1178560.00	? (?)
	aar	?	25859.00	? (?)
	apk (aggressive)	?	393472.00	? (?)
firebase-database-collection	apk (release)	?	912665.00	? (?)
	aar	?	34214.00	? (?)
	apk (aggressive)	?	313605.00	? (?)
firebase-firestore	apk (release)	?	3139586.00	? (?)
	aar	?	1067163.00	? (?)
	apk (aggressive)	?	443174.00	? (?)
base	apk (release)	?	8754.00	? (?)
base	apk (aggressive)	?	10678.00	? (?)

Metric Unit: byte

Test Logs

https://android-ci.firebaseopensource.com/view/gcs/android-ci/pr-logs/pull/firebase_firebase-android-sdk/1377/binary-size/1242853694798565376

vkryachko · 2020-03-20T16:51:39Z

Could the parsing logic be done in fireci?

The motivation for fireci was to:
keep any ci related postprocessing logic separate from the gradle build with the intent to avoid dependency creep in the main build which, in addition to build slowdown, can cause classpath issues.

yifanyang · 2020-03-20T16:56:43Z

I'm not sure I understand. What do you mean by doing it in fireci?

vkryachko · 2020-03-20T17:06:48Z

You can extend https://github.com/firebase/firebase-android-sdk/tree/master/ci/fireci with custom commands, via fireciplugins directory.

So in your case you could invoke gradle and then do postprocessing on the expected output of the build.

Calculate and upload coverage reports to our own Metrics Service instead of Codecov.

yifanyang · 2020-03-24T18:46:19Z

/test fireci

yifanyang · 2020-03-24T18:50:46Z

/test smoke-tests

vkryachko · 2020-03-24T22:34:10Z

ci/fireci/fireciplugins/coverage.py

+
+
+def construct_request_header():
+  result = subprocess.run(['gcloud', 'auth', 'print-identity-token'], stdout=subprocess.PIPE)


Pls add:

Suggested change

result = subprocess.run(['gcloud', 'auth', 'print-identity-token'], stdout=subprocess.PIPE)

result = subprocess.run(['gcloud', 'auth', 'print-identity-token'], stdout=subprocess.PIPE, check=True)

Otherwise you won't know if gcloud fails

additionally, this seems a good candidate for a cli flag as well with a default that makes the subprocess call, that way if one has a token it can be passed in without an additonal call to gcloud

vkryachko · 2020-03-24T22:35:30Z

ci/fireci/fireciplugins/coverage.py

+  endpoint = construct_request_endpoint()
+  headers = construct_request_header()
+
+  print('Posting to the metrics service ...')


pls use logging.getLogger('fireci.coverage') throughout.

vkryachko · 2020-03-24T22:42:20Z

ci/fireci/fireciplugins/coverage.py

+  return 0
+
+
+def find_prow_job_link():


Consider making this a flag and have its default point to this function.

vkryachko · 2020-03-24T22:44:37Z

ci/fireci/fireciplugins/coverage.py

+
+
+def post_request(data):
+  metrics_service_url = os.getenv('METRICS_SERVICE_URL')


consider making this an option with envvar="METRICS_SERVICE_URL"

By "option", do you mean a @click.option?

Oh, I get it.

https://click.palletsprojects.com/en/7.x/arguments/#environment-variables

yifanyang requested a review from vkryachko March 20, 2020 16:33

googlebot added the cla: yes Override cla label Mar 20, 2020

google-oss-bot added the size/L label Mar 20, 2020

yifanyang added 2 commits March 23, 2020 22:18

Improve coverage report format in GitHub pull requests.

63c88cf

Calculate and upload coverage reports to our own Metrics Service instead of Codecov.

Coverage parsing and uploading in fireci.

1366c0c

yifanyang force-pushed the yifany/coverage branch from c50dc97 to 1366c0c Compare March 24, 2020 18:13

Revert changes in the uploader.

964a6a1

vkryachko reviewed Mar 24, 2020

View reviewed changes

yifanyang added 2 commits March 24, 2020 15:47

Do not fail fast on coverage tasks.

ed32d1b

Address feedback.

aa1370d

vkryachko approved these changes Mar 25, 2020

View reviewed changes

yifanyang merged commit fd3abfc into master Mar 25, 2020

yifanyang deleted the yifany/coverage branch March 25, 2020 17:05

firebase locked and limited conversation to collaborators Apr 25, 2020



		def construct_request_header():
		result = subprocess.run(['gcloud', 'auth', 'print-identity-token'], stdout=subprocess.PIPE)



		def post_request(data):
		metrics_service_url = os.getenv('METRICS_SERVICE_URL')

Improve coverage report format in GitHub pull requests. #1377

Improve coverage report format in GitHub pull requests. #1377

Uh oh!

Conversation

yifanyang commented Mar 20, 2020

Uh oh!

google-oss-bot commented Mar 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Binary Size Report

Affected SDKs

Test Logs

Uh oh!

vkryachko commented Mar 20, 2020

Uh oh!

yifanyang commented Mar 20, 2020

Uh oh!

vkryachko commented Mar 20, 2020

Uh oh!

yifanyang commented Mar 24, 2020

Uh oh!

yifanyang commented Mar 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

google-oss-bot commented Mar 20, 2020 •

edited

Loading