Revert "Using protobuf instead of ciyaml directly." (#2220)
diff --git a/app_dart/lib/src/foundation/utils.dart b/app_dart/lib/src/foundation/utils.dart
index 40dcfd6..4183e09 100644
--- a/app_dart/lib/src/foundation/utils.dart
+++ b/app_dart/lib/src/foundation/utils.dart
@@ -165,21 +165,14 @@
final List<String> noOwnerBuilders = <String>[];
final YamlMap? ciYaml = loadYaml(ciYamlContent) as YamlMap?;
final pb.SchedulerConfig unCheckedSchedulerConfig = pb.SchedulerConfig()..mergeFromProto3Json(ciYaml);
- final CiYaml ciYamlFromProto = CiYaml(
+ final pb.SchedulerConfig schedulerConfig = CiYaml(
slug: Config.flutterSlug,
branch: Config.defaultBranch(Config.flutterSlug),
config: unCheckedSchedulerConfig,
- );
-
- final pb.SchedulerConfig schedulerConfig = ciYamlFromProto.config;
-
+ ).config;
for (pb.Target target in schedulerConfig.targets) {
final String builder = target.name;
- final String? owner = getTestOwnership(
- builder,
- getTypeForBuilder(builder, ciYamlFromProto),
- testOwnersContent,
- ).owner;
+ final String? owner = getTestOwnership(builder, getTypeForBuilder(builder, ciYaml!), testOwnersContent).owner;
print('$builder: $owner');
if (owner == null) {
noOwnerBuilders.add(builder);
diff --git a/app_dart/lib/src/model/ci_yaml/target.dart b/app_dart/lib/src/model/ci_yaml/target.dart
index 316e898..3a70770 100644
--- a/app_dart/lib/src/model/ci_yaml/target.dart
+++ b/app_dart/lib/src/model/ci_yaml/target.dart
@@ -89,14 +89,6 @@
return BatchPolicy();
}
- /// Get the tags from the defined properties in the ci.
- ///
- /// Return an empty list if no tags are found.
- List<String> get getTags {
- Map<String, Object> properties = getProperties();
- return (properties.containsKey('tags')) ? (properties['tags'] as List).map((e) => e as String).toList() : [];
- }
-
/// Gets the assembled properties for this [pb.Target].
///
/// Target properties are prioritized in:
diff --git a/app_dart/lib/src/request_handlers/check_flaky_builders.dart b/app_dart/lib/src/request_handlers/check_flaky_builders.dart
index 82e7a10..5b40997 100644
--- a/app_dart/lib/src/request_handlers/check_flaky_builders.dart
+++ b/app_dart/lib/src/request_handlers/check_flaky_builders.dart
@@ -74,9 +74,8 @@
slug,
kTestOwnerPath,
);
-
for (final _BuilderInfo info in eligibleBuilders) {
- final BuilderType type = getTypeForBuilder(info.name, ciYaml);
+ final BuilderType type = getTypeForBuilder(info.name, loadYaml(ciContent) as YamlMap);
final TestOwnership testOwnership = getTestOwnership(info.name!, type, testOwnerContent);
final List<BuilderRecord> builderRecords =
await bigquery.listRecentBuildRecordsForBuilder(kBigQueryProjectId, builder: info.name, limit: kRecordNumber);
@@ -155,8 +154,6 @@
if (nameToExistingPRs.containsKey(builder)) {
continue;
}
-
- //TODO (ricardoamador): Refactor this so we don't need to parse the entire yaml looking for commented issues, https://github.com/flutter/flutter/issues/113232
int builderLineNumber = lines.indexWhere((String line) => line.contains('name: $builder')) + 1;
while (builderLineNumber < lines.length && !lines[builderLineNumber].contains('name:')) {
if (lines[builderLineNumber].contains('$kCiYamlTargetIsFlakyKey:')) {
diff --git a/app_dart/lib/src/request_handlers/file_flaky_issue_and_pr.dart b/app_dart/lib/src/request_handlers/file_flaky_issue_and_pr.dart
index 1decf11..65e727b 100644
--- a/app_dart/lib/src/request_handlers/file_flaky_issue_and_pr.dart
+++ b/app_dart/lib/src/request_handlers/file_flaky_issue_and_pr.dart
@@ -57,7 +57,7 @@
if (statistic.flakyRate < _threshold) {
continue;
}
- final BuilderType type = getTypeForBuilder(statistic.name, ciYaml);
+ final BuilderType type = getTypeForBuilder(statistic.name, ci!);
await _fileIssueAndPR(
gitHub,
slug,
@@ -65,7 +65,7 @@
statistic: statistic,
existingIssue: nameToExistingIssue[statistic.name],
existingPullRequest: nameToExistingPR[statistic.name],
- isMarkedFlaky: _getIsMarkedFlaky(statistic.name, ci!),
+ isMarkedFlaky: _getIsMarkedFlaky(statistic.name, ci),
type: type,
ownership: getTestOwnership(statistic.name, type, testOwnerContent)),
);
diff --git a/app_dart/lib/src/request_handlers/flaky_handler_utils.dart b/app_dart/lib/src/request_handlers/flaky_handler_utils.dart
index 0863779..61ee372 100644
--- a/app_dart/lib/src/request_handlers/flaky_handler_utils.dart
+++ b/app_dart/lib/src/request_handlers/flaky_handler_utils.dart
@@ -5,9 +5,8 @@
import 'dart:convert';
import 'dart:core';
-import 'package:cocoon_service/ci_yaml.dart';
-import 'package:collection/collection.dart';
import 'package:github/github.dart';
+import 'package:yaml/yaml.dart';
import '../service/bigquery.dart';
import '../service/github_service.dart';
@@ -296,8 +295,8 @@
}
/// Looks up the owner of a builder in TESTOWNERS file.
-TestOwnership getTestOwnership(String targetName, BuilderType type, String testOwnersContent) {
- final String testName = _getTestNameFromTargetName(targetName);
+TestOwnership getTestOwnership(String builderName, BuilderType type, String testOwnersContent) {
+ final String testName = _getTestNameFromBuilderName(builderName);
String? owner;
Team? team;
switch (type) {
@@ -419,12 +418,11 @@
/// Gets the [BuilderType] of the builder by looking up the information in the
/// ci.yaml.
-BuilderType getTypeForBuilder(String? targetName, CiYaml ciYaml) {
- final List<String>? tags = _getTags(targetName, ciYaml);
- if (tags == null || tags.isEmpty) {
+BuilderType getTypeForBuilder(String? builderName, YamlMap ci) {
+ final List<dynamic>? tags = _getTags(builderName, ci);
+ if (tags == null) {
return BuilderType.unknown;
}
-
bool hasFrameworkTag = false;
bool hasHostOnlyTag = false;
// If tags contain 'shard', it must be a shard test.
@@ -432,7 +430,7 @@
// If tags contain 'firebaselab`, it must be a firebase tests.
// Otherwise, it is framework host only test if its tags contain both
// 'framework' and 'hostonly'.
- for (String tag in tags) {
+ for (dynamic tag in tags) {
if (tag == kCiYamlTargetTagsFirebaselab) {
return BuilderType.firebaselab;
} else if (tag == kCiYamlTargetTagsShard) {
@@ -448,19 +446,21 @@
return hasFrameworkTag && hasHostOnlyTag ? BuilderType.frameworkHostOnly : BuilderType.unknown;
}
-List<String>? _getTags(String? targetName, CiYaml ciYaml) {
- List<Target> allTargets = ciYaml.presubmitTargets;
- allTargets.addAll(ciYaml.postsubmitTargets);
- Target? target = allTargets.firstWhereOrNull((element) => element.value.name == targetName);
+List<dynamic>? _getTags(String? builderName, YamlMap ci) {
+ final YamlList targets = ci[kCiYamlTargetsKey] as YamlList;
+ final YamlMap? target = targets.firstWhere(
+ (dynamic element) => element[kCiYamlTargetNameKey] == builderName,
+ orElse: () => null,
+ ) as YamlMap?;
if (target == null) {
return null;
}
- return target.getTags;
+ return jsonDecode(target[kCiYamlPropertiesKey][kCiYamlTargetTagsKey] as String) as List<dynamic>?;
}
-String _getTestNameFromTargetName(String targetName) {
+String _getTestNameFromBuilderName(String builderName) {
// The builder names is in the format '<platform> <test name>'.
- final List<String> words = targetName.split(' ');
+ final List<String> words = builderName.split(' ');
return words.length < 2 ? words[0] : words[1];
}
diff --git a/app_dart/lib/src/request_handlers/update_existing_flaky_issues.dart b/app_dart/lib/src/request_handlers/update_existing_flaky_issues.dart
index a25caf0..2c1fe42 100644
--- a/app_dart/lib/src/request_handlers/update_existing_flaky_issues.dart
+++ b/app_dart/lib/src/request_handlers/update_existing_flaky_issues.dart
@@ -28,34 +28,26 @@
const UpdateExistingFlakyIssue({
required super.config,
required super.authenticationProvider,
- @visibleForTesting this.ciYaml,
});
static const String kThresholdKey = 'threshold';
static const int kFreshPeriodForOpenFlake = 7; // days
- final CiYaml? ciYaml;
-
@override
Future<Body> get() async {
final RepositorySlug slug = Config.flutterSlug;
final GithubService gitHub = config.createGithubServiceWithToken(await config.githubOAuthToken);
final BigqueryService bigquery = await config.createBigQueryService();
-
- CiYaml? localCiYaml = ciYaml;
- if (localCiYaml == null) {
- final YamlMap? ci = loadYaml(await gitHub.getFileContent(
- slug,
- kCiYamlPath,
- )) as YamlMap?;
- final pb.SchedulerConfig unCheckedSchedulerConfig = pb.SchedulerConfig()..mergeFromProto3Json(ci);
- localCiYaml = CiYaml(
- slug: slug,
- branch: Config.defaultBranch(slug),
- config: unCheckedSchedulerConfig,
- );
- }
-
+ final YamlMap? ci = loadYaml(await gitHub.getFileContent(
+ slug,
+ kCiYamlPath,
+ )) as YamlMap?;
+ final pb.SchedulerConfig unCheckedSchedulerConfig = pb.SchedulerConfig()..mergeFromProto3Json(ci);
+ final CiYaml ciYaml = CiYaml(
+ slug: slug,
+ branch: Config.defaultBranch(slug),
+ config: unCheckedSchedulerConfig,
+ );
final List<BuilderStatistic> prodBuilderStatisticList =
await bigquery.listBuilderStatistic(kBigQueryProjectId, bucket: 'prod');
final List<BuilderStatistic> stagingBuilderStatisticList =
@@ -64,7 +56,7 @@
await _updateExistingFlakyIssue(
gitHub,
slug,
- localCiYaml,
+ ciYaml,
prodBuilderStatisticList: prodBuilderStatisticList,
stagingBuilderStatisticList: stagingBuilderStatisticList,
nameToExistingIssue: nameToExistingIssue,
@@ -87,7 +79,6 @@
required Bucket bucket,
required BuilderStatistic statistic,
required Issue existingIssue,
- required CiYaml ciYaml,
}) async {
if (DateTime.now().difference(existingIssue.createdAt!) < const Duration(days: kFreshPeriodForOpenFlake)) {
return;
@@ -97,12 +88,17 @@
await gitHub.createComment(slug, issueNumber: existingIssue.number, body: updateBuilder.issueUpdateComment);
await gitHub.replaceLabelsForIssue(slug, issueNumber: existingIssue.number, labels: updateBuilder.issueLabels);
if (existingIssue.assignee == null && !updateBuilder.isBelow) {
+ final String ciContent = await gitHub.getFileContent(
+ slug,
+ kCiYamlPath,
+ );
final String testOwnerContent = await gitHub.getFileContent(
slug,
kTestOwnerPath,
);
- final String? testOwner =
- getTestOwnership(statistic.name, getTypeForBuilder(statistic.name, ciYaml), testOwnerContent).owner;
+ final String? testOwner = getTestOwnership(
+ statistic.name, getTypeForBuilder(statistic.name, loadYaml(ciContent) as YamlMap), testOwnerContent)
+ .owner;
if (testOwner != null) {
await gitHub.assignIssue(slug, issueNumber: existingIssue.number, assignee: testOwner);
}
@@ -137,14 +133,8 @@
builderFlakyMap.containsKey(statistic.name) &&
// ignore: iterable_contains_unrelated_type
!ignoreFlakyMap.containsKey(statistic.name)) {
- await _addCommentToExistingIssue(
- gitHub,
- slug,
- bucket: Bucket.prod,
- statistic: statistic,
- existingIssue: nameToExistingIssue[statistic.name]!,
- ciYaml: ciYaml,
- );
+ await _addCommentToExistingIssue(gitHub, slug,
+ bucket: Bucket.prod, statistic: statistic, existingIssue: nameToExistingIssue[statistic.name]!);
}
}
// For all staging builder stats, updates any existing flaky bug.
@@ -153,14 +143,8 @@
builderFlakyMap[statistic.name] == true &&
// ignore: iterable_contains_unrelated_type
!ignoreFlakyMap.containsKey(statistic.name)) {
- await _addCommentToExistingIssue(
- gitHub,
- slug,
- bucket: Bucket.staging,
- statistic: statistic,
- existingIssue: nameToExistingIssue[statistic.name]!,
- ciYaml: ciYaml,
- );
+ await _addCommentToExistingIssue(gitHub, slug,
+ bucket: Bucket.staging, statistic: statistic, existingIssue: nameToExistingIssue[statistic.name]!);
}
}
}
diff --git a/app_dart/test/model/ci_yaml/target_test.dart b/app_dart/test/model/ci_yaml/target_test.dart
index 189385b..cd2b4ca 100644
--- a/app_dart/test/model/ci_yaml/target_test.dart
+++ b/app_dart/test/model/ci_yaml/target_test.dart
@@ -105,34 +105,6 @@
});
});
- test('tags are parsed from within properties', () {
- final Target target = generateTarget(
- 1,
- platform: 'Linux_build_test',
- platformProperties: <String, String>{
- // This should be overrided by the target specific property
- 'android_sdk': 'abc',
- },
- properties: <String, String>{'xcode': '12abc', 'tags': '["devicelab", "android", "linux"]'},
- );
- expect(target.getTags, ['devicelab', 'android', 'linux']);
- });
-
- test('we do not blow up if tags are not present', () {
- final Target target = generateTarget(
- 1,
- platform: 'Linux_build_test',
- platformProperties: <String, String>{
- // This should be overrided by the target specific property
- 'android_sdk': 'abc',
- },
- properties: <String, String>{
- 'xcode': '12abc',
- },
- );
- expect(target.getTags, []);
- });
-
test('platform properties with xcode', () {
final Target target = generateTarget(
1,
diff --git a/app_dart/test/request_handlers/update_existing_flaky_issues_test.dart b/app_dart/test/request_handlers/update_existing_flaky_issues_test.dart
index d8267e7..b2b27c7 100644
--- a/app_dart/test/request_handlers/update_existing_flaky_issues_test.dart
+++ b/app_dart/test/request_handlers/update_existing_flaky_issues_test.dart
@@ -56,6 +56,14 @@
return const Stream<Issue>.empty();
});
+ // when gets the content of .ci.yaml
+ when(mockRepositoriesService.getContents(
+ captureAny,
+ kCiYamlPath,
+ )).thenAnswer((Invocation invocation) {
+ return Future<RepositoryContents>.value(
+ RepositoryContents(file: GitHubFile(content: gitHubEncode(ciYamlContent))));
+ });
// when gets the content of TESTOWNERS
when(mockRepositoriesService.getContents(
captureAny,
@@ -79,7 +87,6 @@
handler = UpdateExistingFlakyIssue(
config: config,
authenticationProvider: auth,
- ciYaml: testCiYaml,
);
});
@@ -319,7 +326,6 @@
)).thenAnswer((Invocation invocation) {
return Future<Response>.value(Response('[]', 200));
});
-
final Map<String, dynamic> result = await utf8.decoder
.bind((await tester.get<Body>(handler)).serialize() as Stream<List<int>>)
.transform(json.decoder)
diff --git a/app_dart/test/request_handlers/update_existing_flaky_issues_test_data.dart b/app_dart/test/request_handlers/update_existing_flaky_issues_test_data.dart
index 5b3c821..f05638b 100644
--- a/app_dart/test/request_handlers/update_existing_flaky_issues_test_data.dart
+++ b/app_dart/test/request_handlers/update_existing_flaky_issues_test_data.dart
@@ -4,11 +4,7 @@
import 'dart:convert';
-import 'package:cocoon_service/src/model/ci_yaml/ci_yaml.dart';
import 'package:cocoon_service/src/service/bigquery.dart';
-import 'package:cocoon_service/src/service/config.dart';
-
-import 'package:cocoon_service/src/model/proto/protos.dart' as pb;
const String expectedSemanticsIntegrationTestIssueComment = '''
[prod pool] current flaky ratio for the past (up to) 100 commits is 50.00%. Flaky number: 3; total number: 10.
@@ -207,82 +203,66 @@
Please follow https://github.com/flutter/flutter/wiki/Reducing-Test-Flakiness#fixing-flaky-tests to fix the flakiness and enable the test back after validating the fix (internal dashboard to validate: go/flutter_test_flakiness).
''';
-final CiYaml testCiYaml = CiYaml(
- slug: Config.flutterSlug,
- branch: Config.defaultBranch(Config.flutterSlug),
- config: pb.SchedulerConfig(
- enabledBranches: <String>[
- Config.defaultBranch(Config.flutterSlug),
- ],
- targets: <pb.Target>[
- pb.Target(
- name: 'Mac_android android_semantics_integration_test',
- scheduler: pb.SchedulerSystem.luci,
- presubmit: false,
- properties: <String, String>{
- 'tags': jsonEncode(['devicelab'])
- },
- ),
- pb.Target(
- name: 'Mac_android ignore_myflakiness',
- scheduler: pb.SchedulerSystem.luci,
- presubmit: false,
- properties: <String, String>{
- 'ignore_flakiness': 'true',
- 'tags': jsonEncode(['devicelab']),
- }),
- pb.Target(
- name: 'Linux ci_yaml flutter roller',
- scheduler: pb.SchedulerSystem.luci,
- bringup: true,
- timeout: 30,
- runIf: ['.ci.yaml'],
- recipe: 'infra/ci_yaml',
- properties: <String, String>{
- 'tags': jsonEncode(["framework", "hostonly", "shard"]),
- },
- ),
- pb.Target(
- name: 'Mac build_tests_1_4',
- scheduler: pb.SchedulerSystem.luci,
- recipe: 'flutter/flutter_drone',
- timeout: 60,
- properties: <String, String>{
- 'add_recipes_cq': 'true',
- 'shard': 'build_tests',
- 'subshard': '1_4',
- 'tags': jsonEncode(["framework", "hostonly", "shard"]),
- 'dependencies': jsonEncode([
- {
- 'dependency': 'android_sdk',
- 'version': 'version:29.0',
- },
- {
- 'dependency': 'chrome_and_driver',
- 'version': 'version:84',
- },
- {
- 'dependency': 'xcode',
- 'version': '13a233',
- },
- {
- 'dependency': 'open_jdk',
- 'version': '11',
- },
- {
- 'dependency': 'gems',
- 'version': 'v3.3.14',
- },
- {
- 'dependency': 'goldctl',
- 'version': 'git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603',
- },
- ]),
- },
- ),
- ],
- ),
-);
+const String ciYamlContent = '''
+# Describes the targets run in continuous integration environment.
+#
+# Flutter infra uses this file to generate a checklist of tasks to be performed
+# for every commit.
+#
+# More information at:
+# * https://github.com/flutter/cocoon/blob/master/scheduler/README.md
+enabled_branches:
+ - master
+
+targets:
+ - name: Mac_android android_semantics_integration_test
+ builder: Mac_android android_semantics_integration_test
+ presubmit: false
+ scheduler: luci
+ properties:
+ tags: >
+ ["devicelab"]
+
+ - name: Mac_android ignore_myflakiness
+ builder: Mac_android android_semantics_integration_test
+ presubmit: false
+ scheduler: luci
+ properties:
+ ignore_flakiness: "true"
+ tags: >
+ ["devicelab"]
+
+ - name: Linux ci_yaml flutter roller
+ recipe: infra/ci_yaml
+ bringup: true # TODO(chillers): https://github.com/flutter/flutter/issues/93225
+ timeout: 30
+ properties:
+ tags: >
+ ["framework","hostonly","shard"]
+ scheduler: luci
+ runIf:
+ - .ci.yaml
+
+ - name: Mac build_tests_1_4
+ recipe: flutter/flutter_drone
+ timeout: 60
+ properties:
+ add_recipes_cq: "true"
+ dependencies: >-
+ [
+ {"dependency": "android_sdk", "version": "version:29.0"},
+ {"dependency": "chrome_and_driver", "version": "version:84"},
+ {"dependency": "open_jdk", "version": "11"},
+ {"dependency": "xcode", "version": "13a233"},
+ {"dependency": "gems", "version": "v3.3.14"},
+ {"dependency": "goldctl", "version": "git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603"}
+ ]
+ shard: build_tests
+ subshard: "1_4"
+ tags: >
+ ["framework","hostonly","shard"]
+ scheduler: luci
+''';
const String testOwnersContent = '''
diff --git a/app_dart/test/src/utilities/entity_generators.dart b/app_dart/test/src/utilities/entity_generators.dart
index 197b210..786a1c7 100644
--- a/app_dart/test/src/utilities/entity_generators.dart
+++ b/app_dart/test/src/utilities/entity_generators.dart
@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-import 'package:cocoon_service/ci_yaml.dart';
import 'package:cocoon_service/src/model/appengine/commit.dart';
import 'package:cocoon_service/src/model/appengine/task.dart';
import 'package:cocoon_service/src/model/ci_yaml/target.dart';
diff --git a/dashboard/pubspec.lock b/dashboard/pubspec.lock
index 9dc64c6..e23aa55 100644
--- a/dashboard/pubspec.lock
+++ b/dashboard/pubspec.lock
@@ -323,7 +323,7 @@
name: material_color_utilities
url: "https://pub.dartlang.org"
source: hosted
- version: "0.2.0"
+ version: "0.1.5"
meta:
dependency: transitive
description:
@@ -447,7 +447,7 @@
name: source_span
url: "https://pub.dartlang.org"
source: hosted
- version: "1.9.1"
+ version: "1.9.0"
stack_trace:
dependency: transitive
description:
@@ -461,7 +461,7 @@
name: stream_channel
url: "https://pub.dartlang.org"
source: hosted
- version: "2.1.1"
+ version: "2.1.0"
stream_transform:
dependency: transitive
description:
@@ -489,7 +489,7 @@
name: test_api
url: "https://pub.dartlang.org"
source: hosted
- version: "0.4.14"
+ version: "0.4.12"
timing:
dependency: transitive
description:
@@ -573,7 +573,7 @@
name: vector_math
url: "https://pub.dartlang.org"
source: hosted
- version: "2.1.4"
+ version: "2.1.2"
watcher:
dependency: transitive
description:
@@ -596,5 +596,5 @@
source: hosted
version: "3.1.1"
sdks:
- dart: ">=2.18.0 <3.0.0"
+ dart: ">=2.17.0 <3.0.0"
flutter: ">=2.10.0"
