Skip staging test update to cocoon in test runner (#88835)
diff --git a/dev/devicelab/lib/command/upload_results.dart b/dev/devicelab/lib/command/upload_results.dart index df7f06f..19f4a3a 100644 --- a/dev/devicelab/lib/command/upload_results.dart +++ b/dev/devicelab/lib/command/upload_results.dart
@@ -41,21 +41,14 @@ 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 3d3d380..2fe5d0a 100644 --- a/dev/devicelab/lib/framework/cocoon.dart +++ b/dev/devicelab/lib/framework/cocoon.dart
@@ -75,16 +75,16 @@ 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<void> sendResultsPath({ + Future<void> sendTaskStatus({ String? resultsPath, bool? isTestFlaky, String? gitBranch, @@ -102,8 +102,7 @@ resultsJson['NewStatus'] = testStatus; } resultsJson['TestFlaky'] = isTestFlaky ?? false; - const List<String> supportedBranches = <String>['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 @@ } } + /// Only post-submit tests on `master` are allowed to update in cocoon. + bool _shouldUpdateCocoon(Map<String, dynamic> resultJson) { + const List<String> supportedBranches = <String>['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<void> writeTaskResultToFile({ String? builderName,
diff --git a/dev/devicelab/lib/framework/metrics_center.dart b/dev/devicelab/lib/framework/metrics_center.dart index 83bbf5d..c785e03 100644 --- a/dev/devicelab/lib/framework/metrics_center.dart +++ b/dev/devicelab/lib/framework/metrics_center.dart
@@ -45,6 +45,7 @@ /// ] /// } List<MetricPoint> parse(Map<String, dynamic> resultsJson) { + print('Results to upload to skia perf: $resultsJson'); final List<String> scoreKeys = (resultsJson['BenchmarkScoreKeys'] as List<dynamic>?)?.cast<String>() ?? const <String>[]; final Map<String, dynamic> resultData = @@ -70,8 +71,13 @@ return metricPoints; } -/// Upload test metrics to metrics center. -Future<void> 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<void> 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 92b2e33..097a28b 100644 --- a/dev/devicelab/test/cocoon_test.dart +++ b/dev/devicelab/test/cocoon_test.dart
@@ -132,7 +132,7 @@ '"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 @@ '"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 @@ '"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 @@ '"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 @@ '"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 @@ '"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<ClientException>())); + expect(() => cocoon.sendTaskStatus(resultsPath: resultsPath), throwsA(isA<ClientException>())); }); test('throws client exception on non-200 responses', () async { @@ -310,10 +310,10 @@ '"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<ClientException>())); + expect(() => cocoon.sendTaskStatus(resultsPath: resultsPath), throwsA(isA<ClientException>())); }); - 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 @@ 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); }); });