Skip to content

Commit 0a2d9f5

Browse files
auto-submit[bot]auto-submit[bot]
andauthored
Reverts "Remove Cocoon from dev/devicelab, keeping Skia perf stats upload. (#165749)" (#165754)
<!-- start_original_pr_link --> Reverts: flutter/flutter#165749 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: matanlurey <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: Still passing command-line arguments from recipes that have no effect but cause the runner to crash. <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: matanlurey <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {jtmcdole} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: Partial re-land of flutter/flutter#165628: - Fixes the mistake that the `Cocoon` class did things that well, were not specific to Cocoon. - Renamed to `MetricsResultWriter`, as that is all it does now. --- Closes flutter/flutter#165618. The `devicelab/bin/test_runner.dart upload-metrics` command use to have _two_ responsibilities: - Well, upload test **metrics** (benchmarks) to Skia Perf (it still does that) - Upload test **status** to Cocoon (it did until flutter/flutter#165614) As flutter/flutter#165614 proved, this API predated the current LUCI setup, where Cocoon itself receives task status updates from LUCI, and it turns out this entire time, DeviceLab was making (at best) NOP calls, and at worst, causing crashes and corrupt data (flutter/flutter#165610). <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <[email protected]>
1 parent 05f1980 commit 0a2d9f5

File tree

8 files changed

+770
-247
lines changed

8 files changed

+770
-247
lines changed

dev/devicelab/bin/test_runner.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import 'dart:io';
88
import 'package:args/command_runner.dart';
99

1010
import 'package:flutter_devicelab/command/test.dart';
11-
import 'package:flutter_devicelab/command/upload_metrics.dart';
11+
import 'package:flutter_devicelab/command/upload_results.dart';
1212

1313
final CommandRunner<void> runner =
1414
CommandRunner<void>('devicelab_runner', 'DeviceLab test runner for recording test results')
1515
..addCommand(TestCommand())
16-
..addCommand(UploadMetricsCommand());
16+
..addCommand(UploadResultsCommand());
1717

1818
Future<void> main(List<String> rawArgs) async {
1919
unawaited(

dev/devicelab/lib/command/upload_metrics.dart

Lines changed: 0 additions & 42 deletions
This file was deleted.
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:args/command_runner.dart';
6+
7+
import '../framework/metrics_center.dart';
8+
9+
class UploadResultsCommand extends Command<void> {
10+
UploadResultsCommand() {
11+
argParser.addOption('results-file', help: 'Test results JSON to upload to Cocoon.');
12+
argParser.addOption(
13+
'service-account-token-file',
14+
help: 'Authentication token for uploading results.',
15+
);
16+
argParser.addOption(
17+
'test-flaky',
18+
help: 'Flag to show whether the test is flaky: "True" or "False"',
19+
);
20+
argParser.addOption(
21+
'git-branch',
22+
help:
23+
'[Flutter infrastructure] Git branch of the current commit. LUCI\n'
24+
'checkouts run in detached HEAD state, so the branch must be passed.',
25+
);
26+
argParser.addOption(
27+
'luci-builder',
28+
help: '[Flutter infrastructure] Name of the LUCI builder being run on.',
29+
);
30+
argParser.addOption(
31+
'task-name',
32+
help: '[Flutter infrastructure] Name of the task being run on.',
33+
);
34+
argParser.addOption(
35+
'benchmark-tags',
36+
help: '[Flutter infrastructure] Benchmark tags to surface on Skia Perf',
37+
);
38+
argParser.addOption('test-status', help: 'Test status: Succeeded|Failed');
39+
argParser.addOption('commit-time', help: 'Commit time in UNIX timestamp');
40+
argParser.addOption(
41+
'builder-bucket',
42+
help: '[Flutter infrastructure] Luci builder bucket the test is running in.',
43+
);
44+
}
45+
46+
@override
47+
String get name => 'upload-metrics';
48+
49+
@override
50+
String get description => '[Flutter infrastructure] Upload results data to Cocoon/Skia Perf';
51+
52+
@override
53+
Future<void> run() async {
54+
final String? resultsPath = argResults!['results-file'] as String?;
55+
// final String? serviceAccountTokenFile = argResults!['service-account-token-file'] as String?;
56+
// final String? testFlakyStatus = argResults!['test-flaky'] as String?;
57+
final String? gitBranch = argResults!['git-branch'] as String?;
58+
final String? builderName = argResults!['luci-builder'] as String?;
59+
final String? testStatus = argResults!['test-status'] as String?;
60+
final String? commitTime = argResults!['commit-time'] as String?;
61+
final String? taskName = argResults!['task-name'] as String?;
62+
final String? benchmarkTags = argResults!['benchmark-tags'] as String?;
63+
final String? builderBucket = argResults!['builder-bucket'] as String?;
64+
65+
// Upload metrics to skia perf from test runner when `resultsPath` is specified.
66+
if (resultsPath != null) {
67+
await uploadToSkiaPerf(resultsPath, commitTime, taskName, benchmarkTags);
68+
print('Successfully uploaded metrics to skia perf');
69+
}
70+
71+
print(
72+
'Intentionally skipping /api/update-task-status ($gitBranch/$builderName/$testStatus/$builderBucket) because yjbanov@ said so',
73+
);
74+
// final Cocoon cocoon = Cocoon(serviceAccountTokenPath: serviceAccountTokenFile);
75+
// return cocoon.sendTaskStatus(
76+
// resultsPath: resultsPath,
77+
// isTestFlaky: testFlakyStatus == 'True',
78+
// gitBranch: gitBranch,
79+
// builderName: builderName,
80+
// testStatus: testStatus,
81+
// builderBucket: builderBucket,
82+
// );
83+
}
84+
}

0 commit comments

Comments
 (0)