From 0a2d9f56585824689d26a751f3326bf22865ea11 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Sat, 22 Mar 2025 18:58:47 +0000 Subject: [PATCH] Reverts "Remove Cocoon from `dev/devicelab`, keeping Skia perf stats upload. (#165749)" (#165754) Reverts: flutter/flutter#165749 Initiated by: matanlurey Reason for reverting: Still passing command-line arguments from recipes that have no effect but cause the runner to crash. Original PR Author: matanlurey Reviewed By: {jtmcdole} This change reverts the following previous change: Partial re-land of https://github.com/flutter/flutter/pull/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 https://github.com/flutter/flutter/issues/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 https://github.com/flutter/flutter/pull/165614) As https://github.com/flutter/flutter/pull/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 (https://github.com/flutter/flutter/issues/165610). Co-authored-by: auto-submit[bot] --- dev/devicelab/bin/test_runner.dart | 4 +- dev/devicelab/lib/command/upload_metrics.dart | 42 -- dev/devicelab/lib/command/upload_results.dart | 84 ++++ dev/devicelab/lib/framework/cocoon.dart | 265 +++++++++++ .../lib/framework/metrics_result_writer.dart | 119 ----- dev/devicelab/lib/framework/runner.dart | 6 +- dev/devicelab/test/cocoon_test.dart | 416 ++++++++++++++++++ .../test/metrics_result_writer_test.dart | 81 ---- 8 files changed, 770 insertions(+), 247 deletions(-) delete mode 100644 dev/devicelab/lib/command/upload_metrics.dart create mode 100644 dev/devicelab/lib/command/upload_results.dart create mode 100644 dev/devicelab/lib/framework/cocoon.dart delete mode 100644 dev/devicelab/lib/framework/metrics_result_writer.dart create mode 100644 dev/devicelab/test/cocoon_test.dart delete mode 100644 dev/devicelab/test/metrics_result_writer_test.dart diff --git a/dev/devicelab/bin/test_runner.dart b/dev/devicelab/bin/test_runner.dart index d3f4ed4f6a..7d10c7e4c7 100644 --- a/dev/devicelab/bin/test_runner.dart +++ b/dev/devicelab/bin/test_runner.dart @@ -8,12 +8,12 @@ import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:flutter_devicelab/command/test.dart'; -import 'package:flutter_devicelab/command/upload_metrics.dart'; +import 'package:flutter_devicelab/command/upload_results.dart'; final CommandRunner runner = CommandRunner('devicelab_runner', 'DeviceLab test runner for recording test results') ..addCommand(TestCommand()) - ..addCommand(UploadMetricsCommand()); + ..addCommand(UploadResultsCommand()); Future main(List rawArgs) async { unawaited( diff --git a/dev/devicelab/lib/command/upload_metrics.dart b/dev/devicelab/lib/command/upload_metrics.dart deleted file mode 100644 index 3ba8522377..0000000000 --- a/dev/devicelab/lib/command/upload_metrics.dart +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:args/command_runner.dart'; - -import '../framework/metrics_center.dart'; - -class UploadMetricsCommand extends Command { - UploadMetricsCommand() { - argParser.addOption('results-file', help: 'Test results JSON to upload to Cocoon.'); - argParser.addOption('commit-time', help: 'Commit time in UNIX timestamp'); - argParser.addOption( - 'task-name', - help: '[Flutter infrastructure] Name of the task being run on.', - ); - argParser.addOption( - 'benchmark-tags', - help: '[Flutter infrastructure] Benchmark tags to surface on Skia Perf', - ); - } - - @override - String get name => 'upload-metrics'; - - @override - String get description => '[Flutter infrastructure] Upload results data to Cocoon/Skia Perf'; - - @override - Future run() async { - final String? resultsPath = argResults!['results-file'] as String?; - final String? commitTime = argResults!['commit-time'] as String?; - final String? taskName = argResults!['task-name'] as String?; - final String? benchmarkTags = argResults!['benchmark-tags'] as String?; - - // Upload metrics to skia perf from test runner when `resultsPath` is specified. - if (resultsPath != null) { - await uploadToSkiaPerf(resultsPath, commitTime, taskName, benchmarkTags); - print('Successfully uploaded metrics to skia perf'); - } - } -} diff --git a/dev/devicelab/lib/command/upload_results.dart b/dev/devicelab/lib/command/upload_results.dart new file mode 100644 index 0000000000..9c15645285 --- /dev/null +++ b/dev/devicelab/lib/command/upload_results.dart @@ -0,0 +1,84 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:args/command_runner.dart'; + +import '../framework/metrics_center.dart'; + +class UploadResultsCommand extends Command { + UploadResultsCommand() { + argParser.addOption('results-file', help: 'Test results JSON to upload to Cocoon.'); + argParser.addOption( + 'service-account-token-file', + help: 'Authentication token for uploading results.', + ); + argParser.addOption( + 'test-flaky', + help: 'Flag to show whether the test is flaky: "True" or "False"', + ); + argParser.addOption( + 'git-branch', + help: + '[Flutter infrastructure] Git branch of the current commit. LUCI\n' + 'checkouts run in detached HEAD state, so the branch must be passed.', + ); + argParser.addOption( + 'luci-builder', + help: '[Flutter infrastructure] Name of the LUCI builder being run on.', + ); + argParser.addOption( + 'task-name', + help: '[Flutter infrastructure] Name of the task being run on.', + ); + argParser.addOption( + 'benchmark-tags', + help: '[Flutter infrastructure] Benchmark tags to surface on Skia Perf', + ); + 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 + String get name => 'upload-metrics'; + + @override + String get description => '[Flutter infrastructure] Upload results data to Cocoon/Skia Perf'; + + @override + Future run() async { + final String? resultsPath = argResults!['results-file'] as String?; + // final String? serviceAccountTokenFile = argResults!['service-account-token-file'] as String?; + // final String? testFlakyStatus = argResults!['test-flaky'] as String?; + final String? gitBranch = argResults!['git-branch'] as String?; + final String? builderName = argResults!['luci-builder'] as String?; + 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? benchmarkTags = argResults!['benchmark-tags'] as String?; + final String? builderBucket = argResults!['builder-bucket'] as String?; + + // Upload metrics to skia perf from test runner when `resultsPath` is specified. + if (resultsPath != null) { + await uploadToSkiaPerf(resultsPath, commitTime, taskName, benchmarkTags); + print('Successfully uploaded metrics to skia perf'); + } + + print( + 'Intentionally skipping /api/update-task-status ($gitBranch/$builderName/$testStatus/$builderBucket) because yjbanov@ said so', + ); + // final Cocoon cocoon = Cocoon(serviceAccountTokenPath: serviceAccountTokenFile); + // return cocoon.sendTaskStatus( + // resultsPath: resultsPath, + // isTestFlaky: testFlakyStatus == 'True', + // gitBranch: gitBranch, + // builderName: builderName, + // testStatus: testStatus, + // builderBucket: builderBucket, + // ); + } +} diff --git a/dev/devicelab/lib/framework/cocoon.dart b/dev/devicelab/lib/framework/cocoon.dart new file mode 100644 index 0000000000..0ad00662c8 --- /dev/null +++ b/dev/devicelab/lib/framework/cocoon.dart @@ -0,0 +1,265 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:convert' show Encoding, json; +import 'dart:io'; + +import 'package:file/file.dart'; +import 'package:file/local.dart'; +import 'package:http/http.dart'; +import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; + +import 'task_result.dart'; +import 'utils.dart'; + +typedef ProcessRunSync = + ProcessResult Function( + String, + List, { + Map? environment, + bool includeParentEnvironment, + bool runInShell, + Encoding? stderrEncoding, + Encoding? stdoutEncoding, + String? workingDirectory, + }); + +/// Class for test runner to interact with Flutter's infrastructure service, Cocoon. +/// +/// Cocoon assigns bots to run these devicelab tasks on real devices. +/// To retrieve these results, the test runner needs to send results back so the database can be updated. +class Cocoon { + Cocoon({ + String? serviceAccountTokenPath, + @visibleForTesting Client? httpClient, + @visibleForTesting this.fs = const LocalFileSystem(), + @visibleForTesting this.processRunSync = Process.runSync, + @visibleForTesting this.requestRetryLimit = 5, + @visibleForTesting this.requestTimeoutLimit = 30, + }) : _httpClient = AuthenticatedCocoonClient( + serviceAccountTokenPath, + httpClient: httpClient, + filesystem: fs, + ); + + /// Client to make http requests to Cocoon. + final AuthenticatedCocoonClient _httpClient; + + final ProcessRunSync processRunSync; + + /// Url used to send results to. + static const String baseCocoonApiUrl = 'https://flutter-dashboard.appspot.com/api'; + + /// Threshold to auto retry a failed test. + static const int retryNumber = 2; + + /// Underlying [FileSystem] to use. + final FileSystem fs; + + static final Logger logger = Logger('CocoonClient'); + + @visibleForTesting + final int requestRetryLimit; + + @visibleForTesting + final int requestTimeoutLimit; + + String get commitSha => _commitSha ?? _readCommitSha(); + String? _commitSha; + + /// Parse the local repo for the current running commit. + String _readCommitSha() { + final ProcessResult result = processRunSync('git', ['rev-parse', 'HEAD']); + if (result.exitCode != 0) { + throw CocoonException(result.stderr as String); + } + + return _commitSha = result.stdout as String; + } + + /// Update test status to Cocoon. + /// + /// Flutter infrastructure's workflow is: + /// 1. Run DeviceLab test + /// 2. Request service account token from luci auth (valid for at least 3 minutes) + /// 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 sendTaskStatus({ + String? resultsPath, + bool? isTestFlaky, + String? gitBranch, + String? builderName, + String? testStatus, + String? builderBucket, + }) async { + Map resultsJson = {}; + if (resultsPath != null) { + final File resultFile = fs.file(resultsPath); + resultsJson = json.decode(await resultFile.readAsString()) as Map; + } else { + resultsJson['CommitBranch'] = gitBranch; + resultsJson['CommitSha'] = commitSha; + resultsJson['BuilderName'] = builderName; + resultsJson['NewStatus'] = testStatus; + } + resultsJson['TestFlaky'] = isTestFlaky ?? false; + 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, + maxAttempts: requestRetryLimit, + ); + } + } + + /// Only post-submit tests on `master` are allowed to update in cocoon. + bool _shouldUpdateCocoon(Map resultJson, String builderBucket) { + const List supportedBranches = ['master']; + return supportedBranches.contains(resultJson['CommitBranch']) && builderBucket == 'prod'; + } + + /// Write the given parameters into an update task request and store the JSON in [resultsPath]. + Future writeTaskResultToFile({ + String? builderName, + String? gitBranch, + required TaskResult result, + required String resultsPath, + }) async { + final Map updateRequest = _constructUpdateRequest( + gitBranch: gitBranch, + builderName: builderName, + result: result, + ); + final File resultFile = fs.file(resultsPath); + if (resultFile.existsSync()) { + resultFile.deleteSync(); + } + logger.fine('Writing results: ${json.encode(updateRequest)}'); + resultFile.createSync(); + resultFile.writeAsStringSync(json.encode(updateRequest)); + } + + Map _constructUpdateRequest({ + String? builderName, + required TaskResult result, + String? gitBranch, + }) { + final Map updateRequest = { + 'CommitBranch': gitBranch, + 'CommitSha': commitSha, + 'BuilderName': builderName, + 'NewStatus': result.succeeded ? 'Succeeded' : 'Failed', + }; + logger.fine('Update request: $updateRequest'); + + // Make a copy of result data because we may alter it for validation below. + updateRequest['ResultData'] = result.data; + + final List validScoreKeys = []; + if (result.benchmarkScoreKeys != null) { + for (final String scoreKey in result.benchmarkScoreKeys!) { + final Object score = result.data![scoreKey] as Object; + if (score is num) { + // Convert all metrics to double, which provide plenty of precision + // without having to add support for multiple numeric types in Cocoon. + result.data![scoreKey] = score.toDouble(); + validScoreKeys.add(scoreKey); + } + } + } + updateRequest['BenchmarkScoreKeys'] = validScoreKeys; + + return updateRequest; + } + + Future _sendUpdateTaskRequest(Map postBody) async { + logger.info('Attempting to send update task request to Cocoon.'); + final Map response = await _sendCocoonRequest('update-task-status', postBody); + if (response['Name'] != null) { + logger.info('Updated Cocoon with results from this task'); + } else { + logger.info(response); + logger.severe('Failed to updated Cocoon with results from this task'); + } + } + + /// Make an API request to Cocoon. + Future> _sendCocoonRequest(String apiPath, [dynamic jsonData]) async { + final Uri url = Uri.parse('$baseCocoonApiUrl/$apiPath'); + + /// Retry requests to Cocoon as sometimes there are issues with the servers, such + /// as version changes to the backend, datastore issues, or latency issues. + final Response response = await retry( + () => _httpClient.post(url, body: json.encode(jsonData)), + retryIf: + (Exception e) => e is SocketException || e is TimeoutException || e is ClientException, + maxAttempts: requestRetryLimit, + ); + return json.decode(response.body) as Map; + } +} + +/// [HttpClient] for sending authenticated requests to Cocoon. +class AuthenticatedCocoonClient extends BaseClient { + AuthenticatedCocoonClient( + this._serviceAccountTokenPath, { + @visibleForTesting Client? httpClient, + @visibleForTesting FileSystem? filesystem, + }) : _delegate = httpClient ?? Client(), + _fs = filesystem ?? const LocalFileSystem(); + + /// Authentication token to have the ability to upload and record test results. + /// + /// This is intended to only be passed on automated runs on LUCI post-submit. + final String? _serviceAccountTokenPath; + + /// Underlying [HttpClient] to send requests to. + final Client _delegate; + + /// Underlying [FileSystem] to use. + final FileSystem _fs; + + /// Value contained in the service account token file that can be used in http requests. + String get serviceAccountToken => _serviceAccountToken ?? _readServiceAccountTokenFile(); + String? _serviceAccountToken; + + /// Get [serviceAccountToken] from the given service account file. + String _readServiceAccountTokenFile() { + return _serviceAccountToken = _fs.file(_serviceAccountTokenPath).readAsStringSync().trim(); + } + + @override + Future send(BaseRequest request) async { + request.headers['Service-Account-Token'] = serviceAccountToken; + final StreamedResponse response = await _delegate.send(request); + + if (response.statusCode != 200) { + throw ClientException( + 'AuthenticatedClientError:\n' + ' URI: ${request.url}\n' + ' HTTP Status: ${response.statusCode}\n' + ' Response body:\n' + '${(await Response.fromStream(response)).body}', + request.url, + ); + } + return response; + } +} + +class CocoonException implements Exception { + CocoonException(this.message); + + /// The message to show to the issuer to explain the error. + final String message; + + @override + String toString() => 'CocoonException: $message'; +} diff --git a/dev/devicelab/lib/framework/metrics_result_writer.dart b/dev/devicelab/lib/framework/metrics_result_writer.dart deleted file mode 100644 index 4f008a12a0..0000000000 --- a/dev/devicelab/lib/framework/metrics_result_writer.dart +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'dart:async'; -import 'dart:convert' show Encoding, json; -import 'dart:io'; - -import 'package:file/file.dart'; -import 'package:file/local.dart'; -import 'package:logging/logging.dart'; -import 'package:meta/meta.dart'; - -import 'task_result.dart'; - -/// Class for test runner to write device-lab metrics results for Skia Perf. -interface class MetricsResultWriter { - MetricsResultWriter({ - @visibleForTesting this.fs = const LocalFileSystem(), - @visibleForTesting this.processRunSync = Process.runSync, - }); - - final ProcessResult Function( - String, - List, { - Map? environment, - bool includeParentEnvironment, - bool runInShell, - Encoding? stderrEncoding, - Encoding? stdoutEncoding, - String? workingDirectory, - }) - processRunSync; - - /// Threshold to auto retry a failed test. - static const int retryNumber = 2; - - /// Underlying [FileSystem] to use. - final FileSystem fs; - - static final Logger logger = Logger('CocoonClient'); - - String get commitSha => _commitSha ?? _readCommitSha(); - String? _commitSha; - - /// Parse the local repo for the current running commit. - String _readCommitSha() { - final ProcessResult result = processRunSync('git', ['rev-parse', 'HEAD']); - if (result.exitCode != 0) { - throw CocoonException(result.stderr as String); - } - - return _commitSha = result.stdout as String; - } - - /// Write the given parameters into an update task request and store the JSON in [resultsPath]. - Future writeTaskResultToFile({ - String? builderName, - String? gitBranch, - required TaskResult result, - required String resultsPath, - }) async { - final Map updateRequest = _constructUpdateRequest( - gitBranch: gitBranch, - builderName: builderName, - result: result, - ); - final File resultFile = fs.file(resultsPath); - if (resultFile.existsSync()) { - resultFile.deleteSync(); - } - logger.fine('Writing results: ${json.encode(updateRequest)}'); - resultFile.createSync(); - resultFile.writeAsStringSync(json.encode(updateRequest)); - } - - Map _constructUpdateRequest({ - String? builderName, - required TaskResult result, - String? gitBranch, - }) { - final Map updateRequest = { - 'CommitBranch': gitBranch, - 'CommitSha': commitSha, - 'BuilderName': builderName, - 'NewStatus': result.succeeded ? 'Succeeded' : 'Failed', - }; - logger.fine('Update request: $updateRequest'); - - // Make a copy of result data because we may alter it for validation below. - updateRequest['ResultData'] = result.data; - - final List validScoreKeys = []; - if (result.benchmarkScoreKeys != null) { - for (final String scoreKey in result.benchmarkScoreKeys!) { - final Object score = result.data![scoreKey] as Object; - if (score is num) { - // Convert all metrics to double, which provide plenty of precision - // without having to add support for multiple numeric types in Cocoon. - result.data![scoreKey] = score.toDouble(); - validScoreKeys.add(scoreKey); - } - } - } - updateRequest['BenchmarkScoreKeys'] = validScoreKeys; - - return updateRequest; - } -} - -class CocoonException implements Exception { - CocoonException(this.message); - - /// The message to show to the issuer to explain the error. - final String message; - - @override - String toString() => 'CocoonException: $message'; -} diff --git a/dev/devicelab/lib/framework/runner.dart b/dev/devicelab/lib/framework/runner.dart index 85847a3f22..13dd86825b 100644 --- a/dev/devicelab/lib/framework/runner.dart +++ b/dev/devicelab/lib/framework/runner.dart @@ -10,8 +10,8 @@ import 'package:meta/meta.dart'; import 'package:vm_service/vm_service.dart'; import 'package:vm_service/vm_service_io.dart'; +import 'cocoon.dart'; import 'devices.dart'; -import 'metrics_result_writer.dart'; import 'task_result.dart'; import 'utils.dart'; @@ -49,7 +49,7 @@ Future runTasks( for (final String taskName in taskNames) { TaskResult result = TaskResult.success(null); int failureCount = 0; - while (failureCount <= MetricsResultWriter.retryNumber) { + while (failureCount <= Cocoon.retryNumber) { result = await rerunTask( taskName, deviceId: deviceId, @@ -137,7 +137,7 @@ Future rerunTask( section('Finished task "$taskName"'); if (resultsPath != null) { - final MetricsResultWriter cocoon = MetricsResultWriter(); + final Cocoon cocoon = Cocoon(); await cocoon.writeTaskResultToFile( builderName: luciBuilder, gitBranch: gitBranch, diff --git a/dev/devicelab/test/cocoon_test.dart b/dev/devicelab/test/cocoon_test.dart new file mode 100644 index 0000000000..e0c5b1c9db --- /dev/null +++ b/dev/devicelab/test/cocoon_test.dart @@ -0,0 +1,416 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_devicelab/framework/cocoon.dart'; +import 'package:flutter_devicelab/framework/task_result.dart'; +import 'package:http/http.dart'; +import 'package:http/testing.dart'; + +import 'common.dart'; + +void main() { + late ProcessResult processResult; + ProcessResult runSyncStub( + String executable, + List args, { + Map? environment, + bool includeParentEnvironment = true, + bool runInShell = false, + Encoding? stderrEncoding, + Encoding? stdoutEncoding, + String? workingDirectory, + }) => processResult; + + // Expected test values. + const String commitSha = 'a4952838bf288a81d8ea11edfd4b4cd649fa94cc'; + const String serviceAccountTokenPath = 'test_account_file'; + const String serviceAccountToken = 'test_token'; + + group('Cocoon', () { + late Client mockClient; + late Cocoon cocoon; + late FileSystem fs; + + setUp(() { + fs = MemoryFileSystem(); + mockClient = MockClient((Request request) async => Response('{}', 200)); + + final File serviceAccountFile = fs.file(serviceAccountTokenPath)..createSync(); + serviceAccountFile.writeAsStringSync(serviceAccountToken); + }); + + test('returns expected commit sha', () { + processResult = ProcessResult(1, 0, commitSha, ''); + cocoon = Cocoon( + serviceAccountTokenPath: serviceAccountTokenPath, + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + ); + + expect(cocoon.commitSha, commitSha); + }); + + test('throws exception on git cli errors', () { + processResult = ProcessResult(1, 1, '', ''); + cocoon = Cocoon( + serviceAccountTokenPath: serviceAccountTokenPath, + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + ); + + expect(() => cocoon.commitSha, throwsA(isA())); + }); + + test('writes expected update task json', () async { + processResult = ProcessResult(1, 0, commitSha, ''); + final TaskResult result = TaskResult.fromJson({ + 'success': true, + 'data': {'i': 0, 'j': 0, 'not_a_metric': 'something'}, + 'benchmarkScoreKeys': ['i', 'j'], + }); + + cocoon = Cocoon(fs: fs, processRunSync: runSyncStub); + + const String resultsPath = 'results.json'; + await cocoon.writeTaskResultToFile( + builderName: 'builderAbc', + gitBranch: 'master', + result: result, + resultsPath: resultsPath, + ); + + final String resultJson = fs.file(resultsPath).readAsStringSync(); + const String expectedJson = + '{' + '"CommitBranch":"master",' + '"CommitSha":"$commitSha",' + '"BuilderName":"builderAbc",' + '"NewStatus":"Succeeded",' + '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' + '"BenchmarkScoreKeys":["i","j"]}'; + expect(resultJson, expectedJson); + }); + + test('uploads metrics sends expected post body', () async { + processResult = ProcessResult(1, 0, commitSha, ''); + const String uploadMetricsRequestWithSpaces = + '{"CommitBranch":"master","CommitSha":"a4952838bf288a81d8ea11edfd4b4cd649fa94cc","BuilderName":"builder a b c","NewStatus":"Succeeded","ResultData":{},"BenchmarkScoreKeys":[],"TestFlaky":false}'; + final MockClient client = MockClient((Request request) async { + if (request.body == uploadMetricsRequestWithSpaces) { + return Response('{}', 200); + } + + return Response( + 'Expected: $uploadMetricsRequestWithSpaces\nReceived: ${request.body}', + 500, + ); + }); + cocoon = Cocoon( + fs: fs, + httpClient: client, + processRunSync: runSyncStub, + serviceAccountTokenPath: serviceAccountTokenPath, + requestRetryLimit: 0, + ); + + const String resultsPath = 'results.json'; + const String updateTaskJson = + '{' + '"CommitBranch":"master",' + '"CommitSha":"$commitSha",' + '"BuilderName":"builder a b c",' //ignore: missing_whitespace_between_adjacent_strings + '"NewStatus":"Succeeded",' + '"ResultData":{},' + '"BenchmarkScoreKeys":[]}'; + fs.file(resultsPath).writeAsStringSync(updateTaskJson); + await cocoon.sendTaskStatus(resultsPath: resultsPath); + }); + + test('uploads expected update task payload from results file', () async { + processResult = ProcessResult(1, 0, commitSha, ''); + cocoon = Cocoon( + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + serviceAccountTokenPath: serviceAccountTokenPath, + 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); + await cocoon.sendTaskStatus(resultsPath: resultsPath); + }); + + test('Verify retries for task result upload', () async { + int requestCount = 0; + mockClient = MockClient((Request request) async { + requestCount++; + if (requestCount == 1) { + return Response('{}', 500); + } else { + return Response('{}', 200); + } + }); + + processResult = ProcessResult(1, 0, commitSha, ''); + cocoon = Cocoon( + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + serviceAccountTokenPath: serviceAccountTokenPath, + requestRetryLimit: 3, + ); + + 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); + await cocoon.sendTaskStatus(resultsPath: resultsPath); + }); + + test('Verify timeout and retry for task result upload', () async { + int requestCount = 0; + const int timeoutValue = 2; + mockClient = MockClient((Request request) async { + requestCount++; + if (requestCount == 1) { + await Future.delayed(const Duration(seconds: timeoutValue + 2)); + throw Exception('Should not reach this, because timeout should trigger'); + } else { + return Response('{}', 200); + } + }); + + processResult = ProcessResult(1, 0, commitSha, ''); + cocoon = Cocoon( + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + serviceAccountTokenPath: serviceAccountTokenPath, + requestRetryLimit: 2, + requestTimeoutLimit: timeoutValue, + ); + + 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); + await cocoon.sendTaskStatus(resultsPath: resultsPath); + }); + + test('Verify timeout does not trigger for result upload', () async { + int requestCount = 0; + const int timeoutValue = 2; + mockClient = MockClient((Request request) async { + requestCount++; + if (requestCount == 1) { + await Future.delayed(const Duration(seconds: timeoutValue - 1)); + return Response('{}', 200); + } else { + throw Exception('This iteration should not be reached, since timeout should not happen.'); + } + }); + + processResult = ProcessResult(1, 0, commitSha, ''); + cocoon = Cocoon( + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + serviceAccountTokenPath: serviceAccountTokenPath, + requestRetryLimit: 2, + requestTimeoutLimit: timeoutValue, + ); + + 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); + await cocoon.sendTaskStatus(resultsPath: resultsPath); + }); + + test('Verify failure without retries for task result upload', () async { + int requestCount = 0; + mockClient = MockClient((Request request) async { + requestCount++; + if (requestCount == 1) { + return Response('{}', 500); + } else { + return Response('{}', 200); + } + }); + + processResult = ProcessResult(1, 0, commitSha, ''); + cocoon = Cocoon( + fs: fs, + httpClient: mockClient, + processRunSync: runSyncStub, + serviceAccountTokenPath: serviceAccountTokenPath, + 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); + expect( + () => cocoon.sendTaskStatus(resultsPath: resultsPath), + throwsA(isA()), + ); + }); + + test('throws client exception on non-200 responses', () async { + 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); + expect( + () => cocoon.sendTaskStatus(resultsPath: resultsPath), + throwsA(isA()), + ); + }); + + test('does not upload results on non-supported branches', () async { + // Any network failure would cause the upload 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":"stable",' + '"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); + }); + + test('does not update for staging test', () async { + // Any network failure would cause the upload 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'); + }); + }); + + group('AuthenticatedCocoonClient', () { + late FileSystem fs; + + setUp(() { + fs = MemoryFileSystem(); + final File serviceAccountFile = fs.file(serviceAccountTokenPath)..createSync(); + serviceAccountFile.writeAsStringSync(serviceAccountToken); + }); + + test('reads token from service account file', () { + final AuthenticatedCocoonClient client = AuthenticatedCocoonClient( + serviceAccountTokenPath, + filesystem: fs, + ); + expect(client.serviceAccountToken, serviceAccountToken); + }); + + test('reads token from service account file with whitespace', () { + final File serviceAccountFile = fs.file(serviceAccountTokenPath)..createSync(); + serviceAccountFile.writeAsStringSync('$serviceAccountToken \n'); + final AuthenticatedCocoonClient client = AuthenticatedCocoonClient( + serviceAccountTokenPath, + filesystem: fs, + ); + expect(client.serviceAccountToken, serviceAccountToken); + }); + + test('throws error when service account file not found', () { + final AuthenticatedCocoonClient client = AuthenticatedCocoonClient( + 'idontexist', + filesystem: fs, + ); + expect(() => client.serviceAccountToken, throwsA(isA())); + }); + }); +} diff --git a/dev/devicelab/test/metrics_result_writer_test.dart b/dev/devicelab/test/metrics_result_writer_test.dart deleted file mode 100644 index cc506e6ac7..0000000000 --- a/dev/devicelab/test/metrics_result_writer_test.dart +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2014 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'dart:convert'; -import 'dart:io'; - -import 'package:file/file.dart'; -import 'package:file/memory.dart'; -import 'package:flutter_devicelab/framework/metrics_result_writer.dart'; -import 'package:flutter_devicelab/framework/task_result.dart'; - -import 'common.dart'; - -void main() { - late ProcessResult processResult; - ProcessResult runSyncStub( - String executable, - List args, { - Map? environment, - bool includeParentEnvironment = true, - bool runInShell = false, - Encoding? stderrEncoding, - Encoding? stdoutEncoding, - String? workingDirectory, - }) => processResult; - - // Expected test values. - const String commitSha = 'a4952838bf288a81d8ea11edfd4b4cd649fa94cc'; - - late MetricsResultWriter writer; - late FileSystem fs; - - setUp(() { - fs = MemoryFileSystem(); - }); - - test('returns expected commit sha', () { - processResult = ProcessResult(1, 0, commitSha, ''); - writer = MetricsResultWriter(fs: fs, processRunSync: runSyncStub); - - expect(writer.commitSha, commitSha); - }); - - test('throws exception on git cli errors', () { - processResult = ProcessResult(1, 1, '', ''); - writer = MetricsResultWriter(fs: fs, processRunSync: runSyncStub); - - expect(() => writer.commitSha, throwsA(isA())); - }); - - test('writes expected update task json', () async { - processResult = ProcessResult(1, 0, commitSha, ''); - final TaskResult result = TaskResult.fromJson({ - 'success': true, - 'data': {'i': 0, 'j': 0, 'not_a_metric': 'something'}, - 'benchmarkScoreKeys': ['i', 'j'], - }); - - writer = MetricsResultWriter(fs: fs, processRunSync: runSyncStub); - - const String resultsPath = 'results.json'; - await writer.writeTaskResultToFile( - builderName: 'builderAbc', - gitBranch: 'master', - result: result, - resultsPath: resultsPath, - ); - - final String resultJson = fs.file(resultsPath).readAsStringSync(); - const String expectedJson = - '{' - '"CommitBranch":"master",' - '"CommitSha":"$commitSha",' - '"BuilderName":"builderAbc",' - '"NewStatus":"Succeeded",' - '"ResultData":{"i":0.0,"j":0.0,"not_a_metric":"something"},' - '"BenchmarkScoreKeys":["i","j"]}'; - expect(resultJson, expectedJson); - }); -}