diff --git a/dev/devicelab/lib/command/upload_results.dart b/dev/devicelab/lib/command/upload_results.dart index df7f06fd7d..19f4a3aa4b 100644 --- a/dev/devicelab/lib/command/upload_results.dart +++ b/dev/devicelab/lib/command/upload_results.dart @@ -41,21 +41,14 @@ class UploadResultsCommand extends Command { final String? testStatus = argResults!['test-status'] as String?; final String? commitTime = argResults!['commit-time'] as String?; - // Upload metrics to metrics_center from test runner when `commitTime` is specified. This - // is mainly for testing purpose. - // The upload step will be skipped from cocoon once this is validated. - // TODO(keyong): remove try block to block test when this is validated to work https://github.com/flutter/flutter/issues/88484 - try { - if (commitTime != null) { - await uploadToMetricsCenter(resultsPath, commitTime); - print('Successfully uploaded metrics to metrics center'); - } - } on Exception catch (e, stacktrace) { - print('Uploading metrics failure: $e\n\n$stacktrace'); + // Upload metrics to skia perf from test runner when `resultsPath` is specified. + if (resultsPath != null) { + await uploadToSkiaPerf(resultsPath, commitTime); + print('Successfully uploaded metrics to skia perf'); } final Cocoon cocoon = Cocoon(serviceAccountTokenPath: serviceAccountTokenFile); - return cocoon.sendResultsPath( + return cocoon.sendTaskStatus( resultsPath: resultsPath, isTestFlaky: testFlakyStatus == 'True', gitBranch: gitBranch, diff --git a/dev/devicelab/lib/framework/cocoon.dart b/dev/devicelab/lib/framework/cocoon.dart index 3d3d38070a..2fe5d0aab9 100644 --- a/dev/devicelab/lib/framework/cocoon.dart +++ b/dev/devicelab/lib/framework/cocoon.dart @@ -75,16 +75,16 @@ class Cocoon { return _commitSha = result.stdout as String; } - /// Upload the JSON results in [resultsPath] to Cocoon. + /// Update test status to Cocoon. /// /// Flutter infrastructure's workflow is: - /// 1. Run DeviceLab test, writing results to a known path + /// 1. Run DeviceLab test /// 2. Request service account token from luci auth (valid for at least 3 minutes) - /// 3. Upload results from (1) to Cocoon + /// 3. Update test status from (1) to Cocoon /// /// The `resultsPath` is not available for all tests. When it doesn't show up, we /// need to append `CommitBranch`, `CommitSha`, and `BuilderName`. - Future sendResultsPath({ + Future sendTaskStatus({ String? resultsPath, bool? isTestFlaky, String? gitBranch, @@ -102,8 +102,7 @@ class Cocoon { resultsJson['NewStatus'] = testStatus; } resultsJson['TestFlaky'] = isTestFlaky ?? false; - const List supportedBranches = ['master']; - if (supportedBranches.contains(resultsJson['CommitBranch'])) { + if (_shouldUpdateCocoon(resultsJson)) { await retry( () async => _sendUpdateTaskRequest(resultsJson).timeout(Duration(seconds: requestTimeoutLimit)), retryIf: (Exception e) => e is SocketException || e is TimeoutException || e is ClientException, @@ -112,6 +111,13 @@ class Cocoon { } } + /// Only post-submit tests on `master` are allowed to update in cocoon. + bool _shouldUpdateCocoon(Map resultJson) { + const List supportedBranches = ['master']; + return supportedBranches.contains(resultJson['CommitBranch']) && + !resultJson['BuilderName'].toString().toLowerCase().contains('staging'); + } + /// Write the given parameters into an update task request and store the JSON in [resultsPath]. Future writeTaskResultToFile({ String? builderName, diff --git a/dev/devicelab/lib/framework/metrics_center.dart b/dev/devicelab/lib/framework/metrics_center.dart index 83bbf5d922..c785e0357e 100644 --- a/dev/devicelab/lib/framework/metrics_center.dart +++ b/dev/devicelab/lib/framework/metrics_center.dart @@ -45,6 +45,7 @@ Future connectFlutterDestination() async { /// ] /// } List parse(Map resultsJson) { + print('Results to upload to skia perf: $resultsJson'); final List scoreKeys = (resultsJson['BenchmarkScoreKeys'] as List?)?.cast() ?? const []; final Map resultData = @@ -70,8 +71,13 @@ List parse(Map resultsJson) { return metricPoints; } -/// Upload test metrics to metrics center. -Future uploadToMetricsCenter(String? resultsPath, String? commitTime) async { +/// Upload JSON results to skia perf. +/// +/// Flutter infrastructure's workflow is: +/// 1. Run DeviceLab test, writing results to a known path +/// 2. Request service account token from luci auth (valid for at least 3 minutes) +/// 3. Upload results from (1) to skia perf. +Future uploadToSkiaPerf(String? resultsPath, String? commitTime) async { int commitTimeSinceEpoch; if (resultsPath == null) { return; diff --git a/dev/devicelab/test/cocoon_test.dart b/dev/devicelab/test/cocoon_test.dart index 92b2e33e18..097a28b185 100644 --- a/dev/devicelab/test/cocoon_test.dart +++ b/dev/devicelab/test/cocoon_test.dart @@ -132,7 +132,7 @@ void main() { '"ResultData":{},' '"BenchmarkScoreKeys":[]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - await cocoon.sendResultsPath(resultsPath: resultsPath); + await cocoon.sendTaskStatus(resultsPath: resultsPath); }); test('uploads expected update task payload from results file', () async { @@ -154,7 +154,7 @@ void main() { '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' '"BenchmarkScoreKeys":["i","j"]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - await cocoon.sendResultsPath(resultsPath: resultsPath); + await cocoon.sendTaskStatus(resultsPath: resultsPath); }); test('Verify retries for task result upload', () async { @@ -186,7 +186,7 @@ void main() { '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' '"BenchmarkScoreKeys":["i","j"]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - await cocoon.sendResultsPath(resultsPath: resultsPath); + await cocoon.sendTaskStatus(resultsPath: resultsPath); }); test('Verify timeout and retry for task result upload', () async { @@ -221,7 +221,7 @@ void main() { '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' '"BenchmarkScoreKeys":["i","j"]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - await cocoon.sendResultsPath(resultsPath: resultsPath); + await cocoon.sendTaskStatus(resultsPath: resultsPath); }); test('Verify timeout does not trigger for result upload', () async { @@ -256,7 +256,7 @@ void main() { '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' '"BenchmarkScoreKeys":["i","j"]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - await cocoon.sendResultsPath(resultsPath: resultsPath); + await cocoon.sendTaskStatus(resultsPath: resultsPath); }); test('Verify failure without retries for task result upload', () async { @@ -288,7 +288,7 @@ void main() { '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' '"BenchmarkScoreKeys":["i","j"]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - expect(() => cocoon.sendResultsPath(resultsPath: resultsPath), throwsA(isA())); + expect(() => cocoon.sendTaskStatus(resultsPath: resultsPath), throwsA(isA())); }); test('throws client exception on non-200 responses', () async { @@ -310,10 +310,10 @@ void main() { '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' '"BenchmarkScoreKeys":["i","j"]}'; fs.file(resultsPath).writeAsStringSync(updateTaskJson); - expect(() => cocoon.sendResultsPath(resultsPath: resultsPath), throwsA(isA())); + expect(() => cocoon.sendTaskStatus(resultsPath: resultsPath), throwsA(isA())); }); - test('does not upload results on non-supported branches', () async { + test('does not update on non-supported branches', () async { // Any network failure would cause the upoad to fail mockClient = MockClient((Request request) async => Response('', 500)); @@ -335,7 +335,32 @@ void main() { fs.file(resultsPath).writeAsStringSync(updateTaskJson); // This will fail if it decided to upload results - await cocoon.sendResultsPath(resultsPath: resultsPath); + await cocoon.sendTaskStatus(resultsPath: resultsPath); + }); + + test('does not update for staging test', () async { + // Any network failure would cause the upoad to fail + mockClient = MockClient((Request request) async => Response('', 500)); + + cocoon = Cocoon( + serviceAccountTokenPath: serviceAccountTokenPath, + fs: fs, + httpClient: mockClient, + requestRetryLimit: 0, + ); + + const String resultsPath = 'results.json'; + const String updateTaskJson = '{' + '"CommitBranch":"master",' + '"CommitSha":"$commitSha",' + '"BuilderName":"Linux_staging_test",' + '"NewStatus":"Succeeded",' + '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' + '"BenchmarkScoreKeys":["i","j"]}'; + fs.file(resultsPath).writeAsStringSync(updateTaskJson); + + // This will fail if it decided to upload results + await cocoon.sendTaskStatus(resultsPath: resultsPath); }); });