diff --git a/app_dart/integration_test/validate_all_ci_configs_test.dart b/app_dart/integration_test/validate_all_ci_configs_test.dart index 194cec175e..111eb5f869 100644 --- a/app_dart/integration_test/validate_all_ci_configs_test.dart +++ b/app_dart/integration_test/validate_all_ci_configs_test.dart @@ -4,6 +4,7 @@ import 'dart:io' as io; +import 'package:cocoon_scheduler/scheduler.dart'; import 'package:cocoon_service/cocoon_service.dart'; import 'package:test/test.dart'; import 'package:yaml/yaml.dart'; diff --git a/app_dart/lib/src/model/appengine/task.dart b/app_dart/lib/src/model/appengine/task.dart index 5306c97334..104fee0758 100644 --- a/app_dart/lib/src/model/appengine/task.dart +++ b/app_dart/lib/src/model/appengine/task.dart @@ -2,12 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:cocoon_scheduler/scheduler.dart' as pb; import 'package:gcloud/db.dart'; import 'package:json_annotation/json_annotation.dart'; import 'package:meta/meta.dart'; import '../../service/luci.dart'; -import '../proto/internal/scheduler.pb.dart' as pb; import 'commit.dart'; import 'key_converter.dart'; diff --git a/app_dart/lib/src/model/proto/protos.dart b/app_dart/lib/src/model/proto/protos.dart index e62728bfa6..3e25c58606 100644 --- a/app_dart/lib/src/model/proto/protos.dart +++ b/app_dart/lib/src/model/proto/protos.dart @@ -6,6 +6,5 @@ export 'internal/build_status_response.pb.dart'; export 'internal/commit.pb.dart'; export 'internal/commit_status.pb.dart'; export 'internal/key.pb.dart'; -export 'internal/scheduler.pb.dart'; export 'internal/stage.pb.dart'; export 'internal/task.pb.dart'; diff --git a/app_dart/lib/src/service/luci.dart b/app_dart/lib/src/service/luci.dart index 429c436d73..54c83fa95f 100644 --- a/app_dart/lib/src/service/luci.dart +++ b/app_dart/lib/src/service/luci.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:math'; import 'package:appengine/appengine.dart'; +import 'package:cocoon_scheduler/scheduler.dart'; import 'package:github/github.dart'; import 'package:json_annotation/json_annotation.dart'; import 'package:meta/meta.dart'; @@ -13,7 +14,6 @@ import 'package:retry/retry.dart'; import '../model/appengine/task.dart'; import '../model/luci/buildbucket.dart'; -import '../model/proto/internal/scheduler.pb.dart'; import '../request_handling/api_request_handler.dart'; import 'buildbucket.dart'; import 'config.dart'; diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index b2a3613f96..76e8940901 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -6,7 +6,7 @@ import 'dart:io'; import 'dart:typed_data'; import 'package:appengine/appengine.dart'; -import 'package:cocoon_service/src/model/luci/buildbucket.dart'; +import 'package:cocoon_scheduler/scheduler.dart'; import 'package:gcloud/db.dart'; import 'package:github/github.dart' as github; import 'package:googleapis/bigquery/v2.dart'; @@ -21,7 +21,7 @@ import '../foundation/utils.dart'; import '../model/appengine/commit.dart'; import '../model/appengine/task.dart'; import '../model/github/checks.dart'; -import '../model/proto/protos.dart' show SchedulerConfig, Target; +import '../model/luci/buildbucket.dart'; import '../request_handling/exceptions.dart'; import 'cache_service.dart'; import 'config.dart'; @@ -434,57 +434,3 @@ class Scheduler { } } } - -/// Load [yamlConfig] to [SchedulerConfig] and validate the dependency graph. -SchedulerConfig schedulerConfigFromYaml(YamlMap yamlConfig) { - final SchedulerConfig config = SchedulerConfig(); - config.mergeFromProto3Json(yamlConfig); - _validateSchedulerConfig(config); - - return config; -} - -void _validateSchedulerConfig(SchedulerConfig schedulerConfig) { - if (schedulerConfig.targets.isEmpty) { - throw const FormatException('Scheduler config must have at least 1 target'); - } - - if (schedulerConfig.enabledBranches.isEmpty) { - throw const FormatException('Scheduler config must have at least 1 enabled branch'); - } - - final Map> targetGraph = >{}; - final List exceptions = []; - // Construct [targetGraph]. With a one scan approach, cycles in the graph - // cannot exist as it only works forward. - for (final Target target in schedulerConfig.targets) { - if (targetGraph.containsKey(target.name)) { - exceptions.add('ERROR: ${target.name} already exists in graph'); - } else { - targetGraph[target.name] = []; - // Add edges - if (target.dependencies.isNotEmpty) { - if (target.dependencies.length != 1) { - exceptions - .add('ERROR: ${target.name} has multiple dependencies which is not supported. Use only one dependency'); - } else { - if (target.dependencies.first == target.name) { - exceptions.add('ERROR: ${target.name} cannot depend on itself'); - } else if (targetGraph.containsKey(target.dependencies.first)) { - targetGraph[target.dependencies.first].add(target); - } else { - exceptions.add('ERROR: ${target.name} depends on ${target.dependencies.first} which does not exist'); - } - } - } - } - } - _checkExceptions(exceptions); -} - -void _checkExceptions(List exceptions) { - if (exceptions.isNotEmpty) { - final String fullException = exceptions.reduce((String exception, _) => exception + '\n'); - throw FormatException(fullException); - } -} diff --git a/app_dart/pubspec.lock b/app_dart/pubspec.lock index ce459dc6ac..56474d752b 100644 --- a/app_dart/pubspec.lock +++ b/app_dart/pubspec.lock @@ -141,6 +141,13 @@ packages: url: "https://pub.dartlang.org" source: hosted version: "1.0.1" + cocoon_scheduler: + dependency: "direct main" + description: + path: "../scheduler" + relative: true + source: path + version: "1.0.0" code_builder: dependency: transitive description: @@ -695,7 +702,7 @@ packages: source: hosted version: "0.0.5" yaml: - dependency: "direct main" + dependency: transitive description: name: yaml url: "https://pub.dartlang.org" diff --git a/app_dart/pubspec.yaml b/app_dart/pubspec.yaml index 2914d106ae..645724d191 100644 --- a/app_dart/pubspec.yaml +++ b/app_dart/pubspec.yaml @@ -12,6 +12,8 @@ environment: dependencies: appengine: ^0.11.0 + cocoon_scheduler: + path: ../scheduler collection: ^1.14.11 corsac_jwt: ^0.2.2 crypto: ^2.0.6 @@ -31,7 +33,6 @@ dependencies: neat_cache: ^1.0.1 protobuf: ^1.0.0 truncate: ^2.1.2 - yaml: ^2.1.16 dev_dependencies: build_runner: ^1.0.0 diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index 182992f61a..08fd788c18 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -4,7 +4,6 @@ import 'dart:convert'; -import 'package:cocoon_service/protos.dart' show SchedulerConfig, SchedulerSystem, Target; import 'package:cocoon_service/src/model/appengine/commit.dart'; import 'package:cocoon_service/src/model/appengine/task.dart'; import 'package:cocoon_service/src/model/github/checks.dart' as cocoon_github; @@ -20,7 +19,6 @@ import 'package:github/github.dart'; import 'package:googleapis/bigquery/v2.dart'; import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; -import 'package:yaml/yaml.dart'; import '../model/github/checks_test_data.dart'; import '../src/datastore/fake_config.dart'; @@ -382,179 +380,6 @@ targets: expect(retryRequest.builderId.builder, 'Linux A'); }); }); - - group('scheduler config', () { - test('constructs graph with one target', () { - final YamlMap singleTargetConfig = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - builder: builderA - properties: - test: abc - ''') as YamlMap; - final SchedulerConfig schedulerConfig = schedulerConfigFromYaml(singleTargetConfig); - expect(schedulerConfig.enabledBranches, ['master']); - expect(schedulerConfig.targets.length, 1); - final Target target = schedulerConfig.targets.first; - expect(target.bringup, false); - expect(target.name, 'A'); - expect(target.properties, { - 'test': 'abc', - }); - expect(target.builder, 'builderA'); - expect(target.scheduler, SchedulerSystem.cocoon); - expect(target.testbed, 'linux-vm'); - expect(target.timeout, 30); - }); - - test('throws exception when non-existent scheduler is given', () { - final YamlMap targetWithNonexistentScheduler = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - scheduler: dashatar - ''') as YamlMap; - expect(() => schedulerConfigFromYaml(targetWithNonexistentScheduler), throwsA(isA())); - }); - - test('constructs graph with dependency chain', () { - final YamlMap dependentTargetConfig = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - - name: B - dependencies: - - A - - name: C - dependencies: - - B - ''') as YamlMap; - final SchedulerConfig schedulerConfig = schedulerConfigFromYaml(dependentTargetConfig); - expect(schedulerConfig.targets.length, 3); - final Target a = schedulerConfig.targets.first; - final Target b = schedulerConfig.targets[1]; - final Target c = schedulerConfig.targets[2]; - expect(a.name, 'A'); - expect(b.name, 'B'); - expect(b.dependencies, ['A']); - expect(c.name, 'C'); - expect(c.dependencies, ['B']); - }); - - test('constructs graph with parent with two dependents', () { - final YamlMap twoDependentTargetConfig = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - - name: B1 - dependencies: - - A - - name: B2 - dependencies: - - A - ''') as YamlMap; - final SchedulerConfig schedulerConfig = schedulerConfigFromYaml(twoDependentTargetConfig); - expect(schedulerConfig.targets.length, 3); - final Target a = schedulerConfig.targets.first; - final Target b1 = schedulerConfig.targets[1]; - final Target b2 = schedulerConfig.targets[2]; - expect(a.name, 'A'); - expect(b1.name, 'B1'); - expect(b1.dependencies, ['A']); - expect(b2.name, 'B2'); - expect(b2.dependencies, ['A']); - }); - - test('fails when there are cyclic targets', () { - final YamlMap configWithCycle = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - dependencies: - - B - - name: B - dependencies: - - A - ''') as YamlMap; - expect( - () => schedulerConfigFromYaml(configWithCycle), - throwsA( - isA().having( - (FormatException e) => e.toString(), - 'message', - contains('ERROR: A depends on B which does not exist'), - ), - )); - }); - - test('fails when there are duplicate targets', () { - final YamlMap configWithDuplicateTargets = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - - name: A - ''') as YamlMap; - expect( - () => schedulerConfigFromYaml(configWithDuplicateTargets), - throwsA( - isA().having( - (FormatException e) => e.toString(), - 'message', - contains('ERROR: A already exists in graph'), - ), - )); - }); - - test('fails when there are multiple dependencies', () { - final YamlMap configWithMultipleDependencies = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - - name: B - - name: C - dependencies: - - A - - B - ''') as YamlMap; - expect( - () => schedulerConfigFromYaml(configWithMultipleDependencies), - throwsA( - isA().having( - (FormatException e) => e.toString(), - 'message', - contains('ERROR: C has multiple dependencies which is not supported. Use only one dependency'), - ), - )); - }); - - test('fails when dependency does not exist', () { - final YamlMap configWithMissingTarget = loadYaml(''' -enabled_branches: - - master -targets: - - name: A - dependencies: - - B - ''') as YamlMap; - expect( - () => schedulerConfigFromYaml(configWithMissingTarget), - throwsA( - isA().having( - (FormatException e) => e.toString(), - 'message', - contains('ERROR: A depends on B which does not exist'), - ), - )); - }); - }); }); } diff --git a/app_dart/test/src/service/fake_scheduler.dart b/app_dart/test/src/service/fake_scheduler.dart index bbd3966266..0e84771e43 100644 --- a/app_dart/test/src/service/fake_scheduler.dart +++ b/app_dart/test/src/service/fake_scheduler.dart @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:cocoon_scheduler/models/scheduler.pb.dart'; import 'package:cocoon_service/src/foundation/github_checks_util.dart'; import 'package:cocoon_service/src/model/appengine/commit.dart'; -import 'package:cocoon_service/src/model/proto/internal/scheduler.pb.dart'; import 'package:cocoon_service/src/service/buildbucket.dart'; import 'package:cocoon_service/src/service/cache_service.dart'; import 'package:cocoon_service/src/service/config.dart'; diff --git a/scheduler/.gitignore b/scheduler/.gitignore new file mode 100644 index 0000000000..3c8a157278 --- /dev/null +++ b/scheduler/.gitignore @@ -0,0 +1,6 @@ +# Files and directories created by pub. +.dart_tool/ +.packages + +# Conventional directory for build output. +build/ diff --git a/scheduler/CHANGELOG.md b/scheduler/CHANGELOG.md new file mode 100644 index 0000000000..effe43c82c --- /dev/null +++ b/scheduler/CHANGELOG.md @@ -0,0 +1,3 @@ +## 1.0.0 + +- Initial version. diff --git a/scheduler/README.md b/scheduler/README.md new file mode 100644 index 0000000000..c290132dc8 --- /dev/null +++ b/scheduler/README.md @@ -0,0 +1,3 @@ +# Scheduler + +Logic for Flutter infrastructure tasks being scheduled on physical devices. \ No newline at end of file diff --git a/scheduler/analysis_options.yaml b/scheduler/analysis_options.yaml new file mode 100644 index 0000000000..804c2c61f0 --- /dev/null +++ b/scheduler/analysis_options.yaml @@ -0,0 +1,194 @@ +# Specify analysis options. +# +# Until there are meta linter rules, each desired lint must be explicitly enabled. +# See: https://github.com/dart-lang/linter/issues/288 +# +# For a list of lints, see: http://dart-lang.github.io/linter/lints/ +# See the configuration guide for more +# https://github.com/dart-lang/sdk/tree/master/pkg/analyzer#configuring-the-analyzer +# +# There are other similar analysis options files in the flutter repos, +# which should be kept in sync with this file: +# +# - analysis_options.yaml (this file) +# - packages/flutter/lib/analysis_options_user.yaml +# - https://github.com/flutter/plugins/blob/master/analysis_options.yaml +# - https://github.com/flutter/engine/blob/master/analysis_options.yaml +# +# This file contains the analysis options used by Flutter tools, such as IntelliJ, +# Android Studio, and the `flutter analyze` command. + +analyzer: + strong-mode: + implicit-casts: false + implicit-dynamic: false + errors: + # treat missing required parameters as a warning (not a hint) + missing_required_param: warning + # treat missing returns as a warning (not a hint) + missing_return: warning + # allow having TODOs in the code + todo: ignore + # Ignore analyzer hints for updating pubspecs when using Future or + # Stream and not importing dart:async + # Please see https://github.com/flutter/flutter/pull/24528 for details. + sdk_version_async_exported_from_core: ignore + implicit_dynamic_parameter: info + exclude: + - ".dart_tool/**" + - "**/*.g.dart" + - "**/*.pb.dart" + - "**/*.pbjson.dart" + - "**/*.pbenum.dart" + +linter: + rules: + # these rules are documented on and in the same order as + # the Dart Lint rules page to make maintenance easier + # https://github.com/dart-lang/linter/blob/master/example/all.yaml + - always_declare_return_types + - always_put_control_body_on_new_line + # - always_put_required_named_parameters_first # we prefer having parameters in the same order as fields https://github.com/flutter/flutter/issues/10219 + - always_require_non_null_named_parameters + - always_specify_types + - annotate_overrides + # - avoid_annotating_with_dynamic # conflicts with always_specify_types + # - avoid_as # incompatible with implicit-casts: false + - avoid_bool_literals_in_conditional_expressions + # - avoid_catches_without_on_clauses # we do this commonly + # - avoid_catching_errors # we do this commonly + - avoid_classes_with_only_static_members + # - avoid_double_and_int_checks # only useful when targeting JS runtime + - avoid_empty_else + - avoid_field_initializers_in_const_classes + - avoid_function_literals_in_foreach_calls + # - avoid_implementing_value_types # not yet tested + - avoid_init_to_null + # - avoid_js_rounded_ints # only useful when targeting JS runtime + - avoid_null_checks_in_equality_operators + # - avoid_positional_boolean_parameters # not yet tested + # - avoid_private_typedef_functions # we prefer having typedef (discussion in https://github.com/flutter/flutter/pull/16356) + - avoid_relative_lib_imports + - avoid_renaming_method_parameters + - avoid_return_types_on_setters + # - avoid_returning_null # there are plenty of valid reasons to return null + # - avoid_returning_null_for_future # not yet tested + - avoid_returning_null_for_void + # - avoid_returning_this # there are plenty of valid reasons to return this + # - avoid_setters_without_getters # not yet tested + # - avoid_shadowing_type_parameters # not yet tested + # - avoid_single_cascade_in_expression_statements # not yet tested + - avoid_slow_async_io + - avoid_types_as_parameter_names + # - avoid_types_on_closure_parameters # conflicts with always_specify_types + - avoid_unused_constructor_parameters + - avoid_void_async + - await_only_futures + - camel_case_types + - cancel_subscriptions + # - cascade_invocations # not yet tested + # - close_sinks # not reliable enough + # - comment_references # blocked on https://github.com/flutter/flutter/issues/20765 + # - constant_identifier_names # needs an opt-out https://github.com/dart-lang/linter/issues/204 + - control_flow_in_finally + - curly_braces_in_flow_control_structures + # - diagnostic_describe_all_properties # not yet tested + - directives_ordering + - empty_catches + - empty_constructor_bodies + - empty_statements + # - file_names # not yet tested + - flutter_style_todos + - hash_and_equals + - implementation_imports + # - invariant_booleans # too many false positives: https://github.com/dart-lang/linter/issues/811 + - iterable_contains_unrelated_type + # - join_return_with_assignment # not yet tested + - library_names + - library_prefixes + # - lines_longer_than_80_chars # not yet tested + - list_remove_unrelated_type + # - literal_only_boolean_expressions # too many false positives: https://github.com/dart-lang/sdk/issues/34181 + - no_adjacent_strings_in_list + - no_duplicate_case_values + - non_constant_identifier_names + # - null_closures # not yet tested + # - omit_local_variable_types # opposite of always_specify_types + # - one_member_abstracts # too many false positives + # - only_throw_errors # https://github.com/flutter/flutter/issues/5792 + - overridden_fields + - package_api_docs + - package_names + - package_prefixed_library_names + # - parameter_assignments # we do this commonly + - prefer_adjacent_string_concatenation + - prefer_asserts_in_initializer_lists + # - prefer_asserts_with_message # not yet tested + - prefer_collection_literals + - prefer_conditional_assignment + - prefer_const_constructors + - prefer_const_constructors_in_immutables + - prefer_const_declarations + - prefer_const_literals_to_create_immutables + # - prefer_constructors_over_static_methods # not yet tested + - prefer_contains + # - prefer_double_quotes # opposite of prefer_single_quotes + - prefer_equal_for_default_values + # - prefer_expression_function_bodies # conflicts with https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#consider-using--for-short-functions-and-methods + - prefer_final_fields + # - prefer_final_in_for_each # not yet tested + - prefer_final_locals + # - prefer_for_elements_to_map_fromIterable # not yet tested + - prefer_foreach + # - prefer_function_declarations_over_variables # not yet tested + - prefer_generic_function_type_aliases + # - prefer_if_elements_to_conditional_expressions # not yet tested + - prefer_if_null_operators + - prefer_initializing_formals + - prefer_inlined_adds + # - prefer_int_literals # not yet tested + # - prefer_interpolation_to_compose_strings # not yet tested + - prefer_is_empty + - prefer_is_not_empty + - prefer_iterable_whereType + # - prefer_mixin # https://github.com/dart-lang/language/issues/32 + # - prefer_null_aware_operators # disable until NNBD, see https://github.com/flutter/flutter/pull/32711#issuecomment-492930932 + - prefer_single_quotes + - prefer_spread_collections + - prefer_typing_uninitialized_variables + - prefer_void_to_null + # - provide_deprecation_message # not yet tested + # - public_member_api_docs # enabled on a case-by-case basis; see e.g. packages/analysis_options.yaml + - recursive_getters + - slash_for_doc_comments + # - sort_child_properties_last # not yet tested + - sort_constructors_first + - sort_pub_dependencies + - sort_unnamed_constructors_first + - test_types_in_equals + - throw_in_finally + # - type_annotate_public_apis # subset of always_specify_types + - type_init_formals + - unawaited_futures + # - unnecessary_await_in_return # not yet tested + - unnecessary_brace_in_string_interps + - unnecessary_const + - unnecessary_getters_setters + # - unnecessary_lambdas # has false positives: https://github.com/dart-lang/linter/issues/498 + - unnecessary_new + - unnecessary_null_aware_assignments + - unnecessary_null_in_if_null_operators + - unnecessary_overrides + - unnecessary_parenthesis + - unnecessary_statements + - unnecessary_this + - unrelated_type_equality_checks + # - unsafe_html # not yet tested + - use_full_hex_values_for_flutter_colors + # - use_function_type_syntax_for_parameters # not yet tested + - use_rethrow_when_possible + # - use_setters_to_change_properties # not yet tested + # - use_string_buffers # has false positives: https://github.com/dart-lang/sdk/issues/34182 + # - use_to_and_as_if_applicable # has false positives, so we prefer to catch this by code-review + - valid_regexps + # - void_checks # not yet tested diff --git a/app_dart/bin/validate_scheduler_config.dart b/scheduler/bin/validate_scheduler_config.dart similarity index 84% rename from app_dart/bin/validate_scheduler_config.dart rename to scheduler/bin/validate_scheduler_config.dart index 97fd5e0c74..1765229c0d 100644 --- a/app_dart/bin/validate_scheduler_config.dart +++ b/scheduler/bin/validate_scheduler_config.dart @@ -4,7 +4,8 @@ import 'dart:io'; -import 'package:cocoon_service/cocoon_service.dart'; +import 'package:cocoon_scheduler/scheduler.dart'; +// ignore: import_of_legacy_library_into_null_safe import 'package:yaml/yaml.dart'; void main(List args) { diff --git a/app_dart/lib/src/model/proto/internal/scheduler.pb.dart b/scheduler/lib/models/scheduler.pb.dart similarity index 100% rename from app_dart/lib/src/model/proto/internal/scheduler.pb.dart rename to scheduler/lib/models/scheduler.pb.dart diff --git a/app_dart/lib/src/model/proto/internal/scheduler.pbenum.dart b/scheduler/lib/models/scheduler.pbenum.dart similarity index 100% rename from app_dart/lib/src/model/proto/internal/scheduler.pbenum.dart rename to scheduler/lib/models/scheduler.pbenum.dart diff --git a/app_dart/lib/src/model/proto/internal/scheduler.pbjson.dart b/scheduler/lib/models/scheduler.pbjson.dart similarity index 100% rename from app_dart/lib/src/model/proto/internal/scheduler.pbjson.dart rename to scheduler/lib/models/scheduler.pbjson.dart diff --git a/app_dart/lib/src/model/proto/internal/scheduler.proto b/scheduler/lib/models/scheduler.proto similarity index 100% rename from app_dart/lib/src/model/proto/internal/scheduler.proto rename to scheduler/lib/models/scheduler.proto diff --git a/scheduler/lib/scheduler.dart b/scheduler/lib/scheduler.dart new file mode 100644 index 0000000000..e72c7d2414 --- /dev/null +++ b/scheduler/lib/scheduler.dart @@ -0,0 +1,62 @@ +// Copyright 2021 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:yaml/yaml.dart'; +import 'models/scheduler.pb.dart'; + +export 'models/scheduler.pb.dart'; + +/// Load [yamlConfig] to [SchedulerConfig] and validate the dependency graph. +SchedulerConfig schedulerConfigFromYaml(YamlMap yamlConfig) { + final SchedulerConfig config = SchedulerConfig(); + config.mergeFromProto3Json(yamlConfig); + _validateSchedulerConfig(config); + + return config; +} + +void _validateSchedulerConfig(SchedulerConfig schedulerConfig) { + if (schedulerConfig.targets.isEmpty) { + throw const FormatException('Scheduler config must have at least 1 target'); + } + + if (schedulerConfig.enabledBranches.isEmpty) { + throw const FormatException('Scheduler config must have at least 1 enabled branch'); + } + + final Map> targetGraph = >{}; + final List exceptions = []; + // Construct [targetGraph]. With a one scan approach, cycles in the graph + // cannot exist as it only works forward. + for (final Target target in schedulerConfig.targets) { + if (targetGraph.containsKey(target.name)) { + exceptions.add('ERROR: ${target.name} already exists in graph'); + } else { + targetGraph[target.name] = []; + // Add edges + if (target.dependencies.isNotEmpty) { + if (target.dependencies.length != 1) { + exceptions + .add('ERROR: ${target.name} has multiple dependencies which is not supported. Use only one dependency'); + } else { + if (target.dependencies.first == target.name) { + exceptions.add('ERROR: ${target.name} cannot depend on itself'); + } else if (targetGraph.containsKey(target.dependencies.first)) { + targetGraph[target.dependencies.first].add(target); + } else { + exceptions.add('ERROR: ${target.name} depends on ${target.dependencies.first} which does not exist'); + } + } + } + } + } + _checkExceptions(exceptions); +} + +void _checkExceptions(List exceptions) { + if (exceptions.isNotEmpty) { + final String fullException = exceptions.reduce((String exception, _) => exception + '\n'); + throw FormatException(fullException); + } +} diff --git a/scheduler/pubspec.lock b/scheduler/pubspec.lock new file mode 100644 index 0000000000..120a68528c --- /dev/null +++ b/scheduler/pubspec.lock @@ -0,0 +1,348 @@ +# Generated by pub +# See https://dart.dev/tools/pub/glossary#lockfile +packages: + _fe_analyzer_shared: + dependency: transitive + description: + name: _fe_analyzer_shared + url: "https://pub.dartlang.org" + source: hosted + version: "14.0.0" + analyzer: + dependency: transitive + description: + name: analyzer + url: "https://pub.dartlang.org" + source: hosted + version: "0.41.2" + args: + dependency: transitive + description: + name: args + url: "https://pub.dartlang.org" + source: hosted + version: "2.1.0" + async: + dependency: transitive + description: + name: async + url: "https://pub.dartlang.org" + source: hosted + version: "2.6.1" + boolean_selector: + dependency: transitive + description: + name: boolean_selector + url: "https://pub.dartlang.org" + source: hosted + version: "2.1.0" + charcode: + dependency: transitive + description: + name: charcode + url: "https://pub.dartlang.org" + source: hosted + version: "1.2.0" + cli_util: + dependency: transitive + description: + name: cli_util + url: "https://pub.dartlang.org" + source: hosted + version: "0.3.0" + collection: + dependency: transitive + description: + name: collection + url: "https://pub.dartlang.org" + source: hosted + version: "1.15.0" + convert: + dependency: transitive + description: + name: convert + url: "https://pub.dartlang.org" + source: hosted + version: "3.0.0" + coverage: + dependency: transitive + description: + name: coverage + url: "https://pub.dartlang.org" + source: hosted + version: "0.15.2" + crypto: + dependency: transitive + description: + name: crypto + url: "https://pub.dartlang.org" + source: hosted + version: "3.0.1" + file: + dependency: transitive + description: + name: file + url: "https://pub.dartlang.org" + source: hosted + version: "6.1.0" + fixnum: + dependency: transitive + description: + name: fixnum + url: "https://pub.dartlang.org" + source: hosted + version: "0.10.11" + glob: + dependency: transitive + description: + name: glob + url: "https://pub.dartlang.org" + source: hosted + version: "2.0.1" + http_multi_server: + dependency: transitive + description: + name: http_multi_server + url: "https://pub.dartlang.org" + source: hosted + version: "3.0.1" + http_parser: + dependency: transitive + description: + name: http_parser + url: "https://pub.dartlang.org" + source: hosted + version: "4.0.0" + io: + dependency: transitive + description: + name: io + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.0" + js: + dependency: transitive + description: + name: js + url: "https://pub.dartlang.org" + source: hosted + version: "0.6.3" + logging: + dependency: transitive + description: + name: logging + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.1" + matcher: + dependency: transitive + description: + name: matcher + url: "https://pub.dartlang.org" + source: hosted + version: "0.12.10" + meta: + dependency: transitive + description: + name: meta + url: "https://pub.dartlang.org" + source: hosted + version: "1.3.0" + mime: + dependency: transitive + description: + name: mime + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.0" + node_preamble: + dependency: transitive + description: + name: node_preamble + url: "https://pub.dartlang.org" + source: hosted + version: "1.4.13" + package_config: + dependency: transitive + description: + name: package_config + url: "https://pub.dartlang.org" + source: hosted + version: "1.9.3" + path: + dependency: transitive + description: + name: path + url: "https://pub.dartlang.org" + source: hosted + version: "1.8.0" + pedantic: + dependency: "direct dev" + description: + name: pedantic + url: "https://pub.dartlang.org" + source: hosted + version: "1.11.0" + pool: + dependency: transitive + description: + name: pool + url: "https://pub.dartlang.org" + source: hosted + version: "1.5.0" + protobuf: + dependency: "direct main" + description: + name: protobuf + url: "https://pub.dartlang.org" + source: hosted + version: "1.1.3" + pub_semver: + dependency: transitive + description: + name: pub_semver + url: "https://pub.dartlang.org" + source: hosted + version: "2.0.0" + shelf: + dependency: transitive + description: + name: shelf + url: "https://pub.dartlang.org" + source: hosted + version: "1.1.2" + shelf_packages_handler: + dependency: transitive + description: + name: shelf_packages_handler + url: "https://pub.dartlang.org" + source: hosted + version: "3.0.0" + shelf_static: + dependency: transitive + description: + name: shelf_static + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.0" + shelf_web_socket: + dependency: transitive + description: + name: shelf_web_socket + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.1" + source_map_stack_trace: + dependency: transitive + description: + name: source_map_stack_trace + url: "https://pub.dartlang.org" + source: hosted + version: "2.1.0" + source_maps: + dependency: transitive + description: + name: source_maps + url: "https://pub.dartlang.org" + source: hosted + version: "0.10.10" + source_span: + dependency: transitive + description: + name: source_span + url: "https://pub.dartlang.org" + source: hosted + version: "1.8.1" + stack_trace: + dependency: transitive + description: + name: stack_trace + url: "https://pub.dartlang.org" + source: hosted + version: "1.10.0" + stream_channel: + dependency: transitive + description: + name: stream_channel + url: "https://pub.dartlang.org" + source: hosted + version: "2.1.0" + string_scanner: + dependency: transitive + description: + name: string_scanner + url: "https://pub.dartlang.org" + source: hosted + version: "1.1.0" + term_glyph: + dependency: transitive + description: + name: term_glyph + url: "https://pub.dartlang.org" + source: hosted + version: "1.2.0" + test: + dependency: "direct dev" + description: + name: test + url: "https://pub.dartlang.org" + source: hosted + version: "1.16.5" + test_api: + dependency: transitive + description: + name: test_api + url: "https://pub.dartlang.org" + source: hosted + version: "0.2.19" + test_core: + dependency: transitive + description: + name: test_core + url: "https://pub.dartlang.org" + source: hosted + version: "0.3.15" + typed_data: + dependency: transitive + description: + name: typed_data + url: "https://pub.dartlang.org" + source: hosted + version: "1.3.0" + vm_service: + dependency: transitive + description: + name: vm_service + url: "https://pub.dartlang.org" + source: hosted + version: "6.2.0" + watcher: + dependency: transitive + description: + name: watcher + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.0" + web_socket_channel: + dependency: transitive + description: + name: web_socket_channel + url: "https://pub.dartlang.org" + source: hosted + version: "2.1.0" + webkit_inspection_protocol: + dependency: transitive + description: + name: webkit_inspection_protocol + url: "https://pub.dartlang.org" + source: hosted + version: "1.0.0" + yaml: + dependency: "direct main" + description: + name: yaml + url: "https://pub.dartlang.org" + source: hosted + version: "2.2.1" +sdks: + dart: ">=2.12.0 <3.0.0" diff --git a/scheduler/pubspec.yaml b/scheduler/pubspec.yaml new file mode 100644 index 0000000000..f567d479c8 --- /dev/null +++ b/scheduler/pubspec.yaml @@ -0,0 +1,15 @@ +name: cocoon_scheduler +description: Flutter infrastructure scheduler +version: 1.0.0 +publish_to: none + +environment: + sdk: '>=2.10.0 <3.0.0' + +dependencies: + protobuf: ^1.0.0 + yaml: ^2.1.16 + +dev_dependencies: + pedantic: ^1.10.0 + test: ^1.16.0 diff --git a/scheduler/test/scheduler_test.dart b/scheduler/test/scheduler_test.dart new file mode 100644 index 0000000000..0cba88ee5e --- /dev/null +++ b/scheduler/test/scheduler_test.dart @@ -0,0 +1,183 @@ +// Copyright 2021 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:cocoon_scheduler/models/scheduler.pb.dart'; +import 'package:cocoon_scheduler/scheduler.dart'; +import 'package:test/test.dart'; +import 'package:yaml/yaml.dart'; + +void main() { + group('scheduler config', () { + test('constructs graph with one target', () { + final YamlMap singleTargetConfig = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + builder: builderA + properties: + test: abc + ''') as YamlMap; + final SchedulerConfig schedulerConfig = schedulerConfigFromYaml(singleTargetConfig); + expect(schedulerConfig.enabledBranches, ['master']); + expect(schedulerConfig.targets.length, 1); + final Target target = schedulerConfig.targets.first; + expect(target.bringup, false); + expect(target.name, 'A'); + expect(target.properties, { + 'test': 'abc', + }); + expect(target.builder, 'builderA'); + expect(target.scheduler, SchedulerSystem.cocoon); + expect(target.testbed, 'linux-vm'); + expect(target.timeout, 30); + }); + + test('throws exception when non-existent scheduler is given', () { + final YamlMap targetWithNonexistentScheduler = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + scheduler: dashatar + ''') as YamlMap; + expect(() => schedulerConfigFromYaml(targetWithNonexistentScheduler), throwsA(isA())); + }); + + test('constructs graph with dependency chain', () { + final YamlMap dependentTargetConfig = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + - name: B + dependencies: + - A + - name: C + dependencies: + - B + ''') as YamlMap; + final SchedulerConfig schedulerConfig = schedulerConfigFromYaml(dependentTargetConfig); + expect(schedulerConfig.targets.length, 3); + final Target a = schedulerConfig.targets.first; + final Target b = schedulerConfig.targets[1]; + final Target c = schedulerConfig.targets[2]; + expect(a.name, 'A'); + expect(b.name, 'B'); + expect(b.dependencies, ['A']); + expect(c.name, 'C'); + expect(c.dependencies, ['B']); + }); + + test('constructs graph with parent with two dependents', () { + final YamlMap twoDependentTargetConfig = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + - name: B1 + dependencies: + - A + - name: B2 + dependencies: + - A + ''') as YamlMap; + final SchedulerConfig schedulerConfig = schedulerConfigFromYaml(twoDependentTargetConfig); + expect(schedulerConfig.targets.length, 3); + final Target a = schedulerConfig.targets.first; + final Target b1 = schedulerConfig.targets[1]; + final Target b2 = schedulerConfig.targets[2]; + expect(a.name, 'A'); + expect(b1.name, 'B1'); + expect(b1.dependencies, ['A']); + expect(b2.name, 'B2'); + expect(b2.dependencies, ['A']); + }); + + test('fails when there are cyclic targets', () { + final YamlMap configWithCycle = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + dependencies: + - B + - name: B + dependencies: + - A + ''') as YamlMap; + expect( + () => schedulerConfigFromYaml(configWithCycle), + throwsA( + isA().having( + (FormatException e) => e.toString(), + 'message', + contains('ERROR: A depends on B which does not exist'), + ), + )); + }); + + test('fails when there are duplicate targets', () { + final YamlMap configWithDuplicateTargets = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + - name: A + ''') as YamlMap; + expect( + () => schedulerConfigFromYaml(configWithDuplicateTargets), + throwsA( + isA().having( + (FormatException e) => e.toString(), + 'message', + contains('ERROR: A already exists in graph'), + ), + )); + }); + + test('fails when there are multiple dependencies', () { + final YamlMap configWithMultipleDependencies = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + - name: B + - name: C + dependencies: + - A + - B + ''') as YamlMap; + expect( + () => schedulerConfigFromYaml(configWithMultipleDependencies), + throwsA( + isA().having( + (FormatException e) => e.toString(), + 'message', + contains('ERROR: C has multiple dependencies which is not supported. Use only one dependency'), + ), + )); + }); + + test('fails when dependency does not exist', () { + final YamlMap configWithMissingTarget = loadYaml(''' +enabled_branches: + - master +targets: + - name: A + dependencies: + - B + ''') as YamlMap; + expect( + () => schedulerConfigFromYaml(configWithMissingTarget), + throwsA( + isA().having( + (FormatException e) => e.toString(), + 'message', + contains('ERROR: A depends on B which does not exist'), + ), + )); + }); + }); +} diff --git a/test_utilities/bin/config_test_runner.sh b/test_utilities/bin/config_test_runner.sh index 6713344bc3..e24b6924b0 100755 --- a/test_utilities/bin/config_test_runner.sh +++ b/test_utilities/bin/config_test_runner.sh @@ -10,7 +10,7 @@ set -ex # Build and analyze echo "Running config tests on $1" -pushd app_dart > /dev/null +pushd scheduler > /dev/null flutter clean pub get diff --git a/tests.yaml b/tests.yaml index e50cfb2832..d21673865a 100644 --- a/tests.yaml +++ b/tests.yaml @@ -23,6 +23,9 @@ tasks: - task: app_dart script: test_utilities/bin/dart_test_runner.sh + - task: scheduler + script: test_utilities/bin/dart_test_runner.sh + - task: app_flutter script: test_utilities/bin/flutter_test_runner.sh