skip staging update (#89137)

dev/devicelab/lib/command/upload_results.dart

@@ -24,6 +24,7 @@ class UploadResultsCommand extends Command<void> {
     argParser.addOption('task-name', help: '[Flutter infrastructure] Name of the task being run on.');
     argParser.addOption('test-status', help: 'Test status: Succeeded|Failed');
     argParser.addOption('commit-time', help: 'Commit time in UNIX timestamp');
+    argParser.addOption('builder-bucket', help: '[Flutter infrastructure] Luci builder bucket the test is running in.');
   }
 
   @override
@@ -42,27 +43,22 @@ class UploadResultsCommand extends Command<void> {
     final String? testStatus = argResults!['test-status'] as String?;
     final String? commitTime = argResults!['commit-time'] as String?;
     final String? taskName = argResults!['task-name'] as String?;
+    final String? builderBucket = argResults!['builder-bucket'] 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, taskName);
-        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, taskName);
+      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,
       builderName: builderName,
       testStatus: testStatus,
+      builderBucket: builderBucket,
     );
   }
 }

dev/devicelab/lib/framework/cocoon.dart

@@ -75,21 +75,22 @@ 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<void> sendResultsPath({
+  Future<void> sendTaskStatus({
     String? resultsPath,
     bool? isTestFlaky,
     String? gitBranch,
     String? builderName,
     String? testStatus,
+    String? builderBucket,
   }) async {
     Map<String, dynamic> resultsJson = <String, dynamic>{};
     if (resultsPath != null) {
@@ -102,8 +103,7 @@ class Cocoon {
       resultsJson['NewStatus'] = testStatus;
     }
     resultsJson['TestFlaky'] = isTestFlaky ?? false;
-    const List<String> supportedBranches = <String>['master'];
-    if (supportedBranches.contains(resultsJson['CommitBranch'])) {
+    if (_shouldUpdateCocoon(resultsJson, builderBucket ?? 'prod')) {
       await retry(
         () async => _sendUpdateTaskRequest(resultsJson).timeout(Duration(seconds: requestTimeoutLimit)),
         retryIf: (Exception e) => e is SocketException || e is TimeoutException || e is ClientException,
@@ -112,6 +112,12 @@ class Cocoon {
     }
   }
 
+  /// Only post-submit tests on `master` are allowed to update in cocoon.
+  bool _shouldUpdateCocoon(Map<String, dynamic> resultJson, String builderBucket) {
+    const List<String> supportedBranches = <String>['master'];
+    return supportedBranches.contains(resultJson['CommitBranch']) && builderBucket == 'prod';
+  }
+
   /// Write the given parameters into an update task request and store the JSON in [resultsPath].
   Future<void> writeTaskResultToFile({
     String? builderName,

dev/devicelab/lib/framework/metrics_center.dart

@@ -45,6 +45,7 @@ Future<FlutterDestination> connectFlutterDestination() async {
 ///     ]
 ///   }
 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 =
@@ -95,8 +96,13 @@ Future<void> upload(
   );
 }
 
-/// Upload test metrics to metrics center.
-Future<void> uploadToMetricsCenter(String? resultsPath, String? commitTime, String? taskName) 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, String? taskName) async {
   int commitTimeSinceEpoch;
   if (resultsPath == null) {
     return;

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<ClientException>()));
+      expect(() => cocoon.sendTaskStatus(resultsPath: resultsPath), throwsA(isA<ClientException>()));
     });
 
     test('throws client exception on non-200 responses', () async {
@@ -310,7 +310,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<ClientException>()));
+      expect(() => cocoon.sendTaskStatus(resultsPath: resultsPath), throwsA(isA<ClientException>()));
     });
 
     test('does not upload results on non-supported branches', () async {
@@ -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":"builderAbc",'
+          '"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, builderBucket: 'staging');
     });
   });